Repository: gin-gonic/gin
Pull request #1919 (2019-05-30 15:12:29)

Bugfix for the FullPath feature
 * worked with more complex situations
 * the original pr not work when add a short route with the same prefix
 to some already added routes

- With pull requests:
  - Open your pull request against `master`
  - Your pull request should have no more than two commits, if not you should squash them.
  - It should pass all tests in the available continuous integrations systems such as TravisCI.
  - You should add/modify tests to cover your proposed code changes.
  - If your pull request contains a new feature, please document it on the README.



bbiao: @zaynetro Here is some fix for your recently commit of the [#1826](https://github.com/gin-gonic/gin/pull/1826)
codecov[bot]: # [Codecov](https://codecov.io/gh/gin-gonic/gin/pull/1919?src=pr&el=h1) Report
> Merging [#1919](https://codecov.io/gh/gin-gonic/gin/pull/1919?src=pr&el=desc) into [master](https://codecov.io/gh/gin-gonic/gin/commit/4b6df417e4bda80a698a64aa085779a7ad1269c0?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/1919/graphs/tree.svg?width=650&token=Qpqxhwery8&height=150&src=pr)](https://codecov.io/gh/gin-gonic/gin/pull/1919?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1919      +/-   ##
==========================================
+ Coverage   98.75%   98.75%   +<.01%     
==========================================
  Files          38       38              
  Lines        2167     2173       +6     
==========================================
+ Hits         2140     2146       +6     
  Misses         15       15              
  Partials       12       12
```


| [Impacted Files](https://codecov.io/gh/gin-gonic/gin/pull/1919?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [tree.go](https://codecov.io/gh/gin-gonic/gin/pull/1919/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/1919?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/1919?src=pr&el=footer). Last update [4b6df41...de00c50](https://codecov.io/gh/gin-gonic/gin/pull/1919?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

zaynetro: Good catch!
thinkerou: the pr branch benchmark data:
```
➜  go-http-routing-benchmark git:(master) ✗ go test -bench=Gin
#GithubAPI Routes: 203
   Gin: 58560 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	        95.1 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Param5       	10000000	       179 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Param20      	 3000000	       533 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParamWrite   	10000000	       175 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubStatic 	10000000	       126 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubParam  	10000000	       231 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubAll    	   30000	     48530 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusStatic  	20000000	        98.9 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusParam   	10000000	       152 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlus2Params 	10000000	       211 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusAll     	 1000000	      2131 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseStatic  	20000000	        99.2 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseParam   	20000000	       102 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Parse2Params 	10000000	       129 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseAll     	  500000	      3344 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_StaticAll    	   50000	     25523 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/julienschmidt/go-http-routing-benchmark	32.030s
```

and master branch benchmark:
```
➜  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	        91.2 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Param5       	10000000	       185 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Param20      	 3000000	       528 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParamWrite   	10000000	       177 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubStatic 	10000000	       124 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubParam  	10000000	       223 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GithubAll    	   30000	     46198 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusStatic  	20000000	        88.5 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusParam   	10000000	       133 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlus2Params 	10000000	       186 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_GPlusAll     	 1000000	      1958 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseStatic  	20000000	        94.2 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseParam   	20000000	       111 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_Parse2Params 	10000000	       135 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_ParseAll     	  500000	      3445 ns/op	       0 B/op	       0 allocs/op
BenchmarkGin_StaticAll    	   50000	     26084 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/julienschmidt/go-http-routing-benchmark	31.207s
```
bbiao: Will this pr be merged ? @thinkerou 
thinkerou: @bbiao also need @appleboy review. thanks!
thinkerou: @appleboy please review the pull request, thanks!

Patch:
diff --git a/tree.go b/tree.go
index 9a789f2..371d5ad 100644
--- a/tree.go
+++ b/tree.go
@@ -128,6 +128,8 @@ func (n *node) addRoute(path string, handlers HandlersChain) {
 	n.priority++
 	numParams := countParams(path)
 
+	parentFullPathIndex := 0
+
 	// non-empty tree
 	if len(n.path) > 0 || len(n.children) > 0 {
 	walk:
@@ -155,7 +157,7 @@ func (n *node) addRoute(path string, handlers HandlersChain) {
 					children:  n.children,
 					handlers:  n.handlers,
 					priority:  n.priority - 1,
-					fullPath:  fullPath,
+					fullPath:  n.fullPath,
 				}
 
 				// Update maxParams (max of all children)
@@ -171,6 +173,7 @@ func (n *node) addRoute(path string, handlers HandlersChain) {
 				n.path = path[:i]
 				n.handlers = nil
 				n.wildChild = false
+				n.fullPath = fullPath[:parentFullPathIndex+i]
 			}
 
 			// Make new node a child of this node
@@ -178,6 +181,7 @@ func (n *node) addRoute(path string, handlers HandlersChain) {
 				path = path[i:]
 
 				if n.wildChild {
+					parentFullPathIndex += len(n.path)
 					n = n.children[0]
 					n.priority++
 
@@ -211,6 +215,7 @@ func (n *node) addRoute(path string, handlers HandlersChain) {
 
 				// slash after param
 				if n.nType == param && c == '/' && len(n.children) == 1 {
+					parentFullPathIndex += len(n.path)
 					n = n.children[0]
 					n.priority++
 					continue walk
@@ -219,6 +224,7 @@ func (n *node) addRoute(path string, handlers HandlersChain) {
 				// Check if a child with the next path byte exists
 				for i := 0; i < len(n.indices); i++ {
 					if c == n.indices[i] {
+						parentFullPathIndex += len(n.path)
 						i = n.incrementChildPrio(i)
 						n = n.children[i]
 						continue walk
@@ -369,6 +375,7 @@ func (n *node) insertChild(numParams uint8, path string, fullPath string, handle
 	// insert remaining path part and handle to the leaf
 	n.path = path[offset:]
 	n.handlers = handlers
+	n.fullPath = fullPath
 }
 
 // nodeValue holds return values of (*Node).getValue method


Test patch:
diff --git a/routes_test.go b/routes_test.go
index 457c923..0c2f9a0 100644
--- a/routes_test.go
+++ b/routes_test.go
@@ -560,10 +560,15 @@ func TestRouteContextHoldsFullPath(t *testing.T) {
 
 	// Test routes
 	routes := []string{
-		"/",
 		"/simple",
 		"/project/:name",
+		"/",
+		"/news/home",
+		"/news",
+		"/simple-two/one",
+		"/simple-two/one-two",
 		"/project/:name/build/*params",
+		"/project/:name/bui",
 	}
 
 	for _, route := range routes {