Fixed #22

Signed-off-by: Vishal Rana <vr@labstack.com>

echo.go

@@ -4,10 +4,11 @@ import (
 	"bytes"
 	"encoding/json"
 	"errors"
-	"fmt"
 	"io"
 	"log"
 	"net/http"
+	"reflect"
+	"runtime"
 	"strings"
 	"sync"
 
@@ -111,7 +112,6 @@ func New() (e *Echo) {
 			Response: &response{},
 			params:   make(Params, e.maxParam),
 			store:    make(store),
-			echo:     e, // TODO: Do we need this?
 		}
 	}
 
@@ -237,7 +237,8 @@ func (e *Echo) URI(h Handler, params ...string) string {
 	uri := new(bytes.Buffer)
 	lp := len(params)
 	n := 0
-	if path, ok := e.uris[fmt.Sprintf("%v", h)]; ok {
+	key := runtime.FuncForPC(reflect.ValueOf(h).Pointer()).Name()
+	if path, ok := e.uris[key]; ok {
 		for i, l := 0, len(path); i < l; i++ {
 			if path[i] == ':' && n < lp {
 				for ; i < l && path[i] != '/'; i++ {
@@ -259,7 +260,8 @@ func (e *Echo) URL(h Handler, params ...string) string {
 }
 
 func (e *Echo) add(method, path string, h Handler) {
-	e.uris[fmt.Sprintf("%v", h)] = path
+	key := runtime.FuncForPC(reflect.ValueOf(h).Pointer()).Name()
+	e.uris[key] = path
 	e.Router.Add(method, e.prefix+path, wrapH(h), e)
 }
 

router.go

@@ -8,13 +8,18 @@ type (
 		echo  *Echo
 	}
 	node struct {
+		typ      ntype
 		label    byte
 		prefix   string
 		parent   *node
 		children children
-		handler  HandlerFunc
+		// pchild   *node    // Param child
-		echo     *Echo
+		// cchild   *node    // Catch-all child
+		handler HandlerFunc
+		pnames  []string
+		echo    *Echo
 	}
+	ntype    uint8
 	children []*node
 	param    struct {
 		Name  string
@@ -23,6 +28,12 @@ type (
 	Params []param
 )
 
+const (
+	stype ntype = iota
+	ptype
+	ctype
+)
+
 func NewRouter(e *Echo) (r *router) {
 	r = &router{
 		trees: make(map[string]*node),
@@ -38,25 +49,34 @@ func NewRouter(e *Echo) (r *router) {
 }
 
 func (r *router) Add(method, path string, h HandlerFunc, echo *Echo) {
+	var pnames []string // Param names
+
 	for i, l := 0, len(path); i < l; i++ {
 		if path[i] == ':' {
-			r.insert(method, path[:i], nil, echo)
+			j := i + 1
+
+			r.insert(method, path[:i], nil, stype, nil, echo)
 			for ; i < l && path[i] != '/'; i++ {
 			}
+
+			pnames = append(pnames, path[j:i])
+			path = path[:j] + path[i:]
+			i, l = j, len(path)
+
 			if i == l {
-				r.insert(method, path[:i], h, echo)
+				r.insert(method, path[:i], h, ptype, pnames, echo)
 				return
 			}
-			r.insert(method, path[:i], nil, echo)
+			r.insert(method, path[:i], nil, ptype, pnames, echo)
 		} else if path[i] == '*' {
-			r.insert(method, path[:i], nil, echo)
+			r.insert(method, path[:i], nil, ctype, nil, echo)
-			r.insert(method, path[:l], h, echo)
+			r.insert(method, path[:l], h, ctype, nil, echo)
 		}
 	}
-	r.insert(method, path, h, echo)
+	r.insert(method, path, h, stype, nil, echo)
 }
 
-func (r *router) insert(method, path string, h HandlerFunc, echo *Echo) {
+func (r *router) insert(method, path string, h HandlerFunc, t ntype, pnames []string, echo *Echo) {
 	cn := r.trees[method] // Current node as root
 	search := path
 
@@ -70,27 +90,33 @@ func (r *router) insert(method, path string, h HandlerFunc, echo *Echo) {
 			cn.label = search[0]
 			cn.prefix = search
 			if h != nil {
+				cn.typ = t
 				cn.handler = h
+				cn.pnames = pnames
 				cn.echo = echo
 			}
 		} else if l < pl {
 			// Split node
-			n := newNode(cn.prefix[l:], cn, cn.children, cn.handler, cn.echo)
+			n := newNode(t, cn.prefix[l:], cn, cn.children, cn.handler, cn.pnames, cn.echo)
 			cn.children = children{n} // Add to parent
 
 			// Reset parent node
+			cn.typ = stype
 			cn.label = cn.prefix[0]
 			cn.prefix = cn.prefix[:l]
 			cn.handler = nil
+			cn.pnames = nil
 			cn.echo = nil
 
 			if l == sl {
 				// At parent node
+				cn.typ = t
 				cn.handler = h
+				cn.pnames = pnames
 				cn.echo = echo
 			} else {
 				// Create child node
-				n = newNode(search[l:], cn, children{}, h, echo)
+				n = newNode(t, search[l:], cn, children{}, h, pnames, echo)
 				cn.children = append(cn.children, n)
 			}
 		} else if l < sl {
@@ -102,12 +128,13 @@ func (r *router) insert(method, path string, h HandlerFunc, echo *Echo) {
 				continue
 			}
 			// Create child node
-			n := newNode(search, cn, children{}, h, echo)
+			n := newNode(t, search, cn, children{}, h, pnames, echo)
 			cn.children = append(cn.children, n)
 		} else {
 			// Node already exists
 			if h != nil {
 				cn.handler = h
+				cn.pnames = pnames
 				cn.echo = echo
 			}
 		}
@@ -115,13 +142,15 @@ func (r *router) insert(method, path string, h HandlerFunc, echo *Echo) {
 	}
 }
 
-func newNode(pfx string, p *node, c children, h HandlerFunc, echo *Echo) (n *node) {
+func newNode(t ntype, pfx string, p *node, c children, h HandlerFunc, pnames []string, echo *Echo) (n *node) {
 	n = &node{
+		typ:      t,
 		label:    pfx[0],
 		prefix:   pfx,
 		parent:   p,
 		children: c,
 		handler:  h,
+		pnames:   pnames,
 		echo:     echo,
 	}
 	return
@@ -136,6 +165,33 @@ func (n *node) findChild(l byte) *node {
 	return nil
 }
 
+func (n *node) findSchild(l byte) *node {
+	for _, c := range n.children {
+		if c.label == l && c.typ == stype {
+			return c
+		}
+	}
+	return nil
+}
+
+func (n *node) findPchild() *node {
+	for _, c := range n.children {
+		if c.typ == ptype {
+			return c
+		}
+	}
+	return nil
+}
+
+func (n *node) findCchild() *node {
+	for _, c := range n.children {
+		if c.typ == ctype {
+			return c
+		}
+	}
+	return nil
+}
+
 // Length of longest common prefix
 func lcp(a, b string) (i int) {
 	max := len(a)
@@ -159,6 +215,9 @@ func (r *router) Find(method, path string, params Params) (h HandlerFunc, echo *
 		if search == "" || search == cn.prefix {
 			// Found
 			h = cn.handler
+			for i := 0; i < len(cn.pnames); i++ {
+				params[i].Name = cn.pnames[i]
+			}
 			echo = cn.echo
 			return
 		}
@@ -173,7 +232,7 @@ func (r *router) Find(method, path string, params Params) (h HandlerFunc, echo *
 		}
 
 		// Static node
-		c = cn.findChild(search[0])
+		c = cn.findSchild(search[0])
 		if c != nil {
 			cn = c
 			continue
@@ -181,13 +240,12 @@ func (r *router) Find(method, path string, params Params) (h HandlerFunc, echo *
 
 		// Param node
 	Param:
-		c = cn.findChild(':')
+		c = cn.findPchild()
 		if c != nil {
 			cn = c
 			i, l := 0, len(search)
 			for ; i < l && search[i] != '/'; i++ {
 			}
-			params[n].Name = cn.prefix[1:]
 			params[n].Value = search[:i]
 			n++
 			search = search[i:]
@@ -195,7 +253,7 @@ func (r *router) Find(method, path string, params Params) (h HandlerFunc, echo *
 		}
 
 		// Catch-all node
-		c = cn.findChild('*')
+		c = cn.findCchild()
 		if c != nil {
 			cn = c
 			p := params[:n+1]

router_test.go

@@ -13,6 +13,15 @@ type route struct {
 	path   string
 }
 
+var (
+	api2 = []route{
+		// Issues
+		{"DELETE", "/repos/:owner/:repo/labels/:name"},
+		{"GET", "/repos/:owner/:repo/issues/:number/labels"},
+		{"POST", "/repos/:owner/:repo/issues/:number/labels"},
+	}
+)
+
 var (
 	params = make(Params, 5)
 	api    = []route{
@@ -316,6 +325,7 @@ func TestRouterTwoParam(t *testing.T) {
 	r.Add(GET, "/users/:uid/files/:fid", func(*Context) error {
 		return nil
 	}, nil)
+
 	h, _ := r.Find(GET, "/users/1/files/1", params)
 	if h == nil {
 		t.Fatal("handler not found")
@@ -470,9 +480,10 @@ func TestRouterConflictingRoute(t *testing.T) {
 		t.Error("param id should be news")
 	}
 
-	//***************//
+	//-----------
-	//   Two level   //
+	// Two level
-	//***************//
+	//-----------
+
 	// Route > /users/new/moon > /users/new/moon
 	h, _ = r.Find(GET, "/users/new/moon", params)
 	if h == nil {
@@ -511,6 +522,63 @@ func TestRouterConflictingRoute(t *testing.T) {
 	}
 }
 
+func TestRouterParamNames(t *testing.T) {
+	r := New().Router
+	b := new(bytes.Buffer)
+
+	// Routes
+	r.Add(GET, "/users", func(*Context) error {
+		b.WriteString("/users")
+		return nil
+	}, nil)
+	r.Add(GET, "/users/:id", func(c *Context) error {
+		return nil
+	}, nil)
+	r.Add(GET, "/users/:uid/files/:fid", func(c *Context) error {
+		return nil
+	}, nil)
+
+	// Route > /users
+	h, _ := r.Find(GET, "/users", params)
+	if h == nil {
+		t.Fatal("handler not found")
+	}
+	h(nil)
+	if b.String() != "/users" {
+		t.Errorf("buffer should be /users")
+	}
+
+	// Route > /users/:id > /users/1
+	h, _ = r.Find(GET, "/users/1", params)
+	if h == nil {
+		t.Fatal("handler not found")
+	}
+	if params[0].Name != "id" {
+		t.Error("param name should be id")
+	}
+	if params[0].Value != "1" {
+		t.Error("param id should be 1")
+	}
+
+	// Route > /users/:uid/files/:fid > /users/1/files/1
+	h, _ = r.Find(GET, "/users/1/files/1", params)
+	if h == nil {
+		t.Fatal("handler not found")
+	}
+	if params[0].Name != "uid" {
+		t.Error("param name should be id")
+	}
+	if params[0].Value != "1" {
+		t.Error("param id should be 1")
+	}
+	if params[1].Name != "fid" {
+		t.Error("param name should be id")
+	}
+	if params[1].Value != "1" {
+		t.Error("param id should be 1")
+	}
+}
+
 func TestRouterAPI(t *testing.T) {
 	r := New().Router
 	for _, route := range api {
@@ -524,11 +592,14 @@ func TestRouterAPI(t *testing.T) {
 			}
 			return nil
 		}, nil)
-
+		c := &Context{params: params}
 		h, _ := r.Find(route.method, route.path, params)
 		if h == nil {
-			t.Errorf("handler not found, method=%s, path=%s", route.method, route.path)
+			t.Fatalf("handler not found, method=%s, path=%s", route.method, route.path)
 		}
+		h(c)
+		// Reset params
+		params = make(Params, 5)
 	}
 }