From c58ec742a98c83724811d0846db175dc9c905a50 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Sat, 9 May 2015 22:06:13 -0700 Subject: [PATCH] Enhanced router priority Signed-off-by: Vishal Rana --- README.md | 3 +- router.go | 58 ++++--- router_test.go | 368 ++++++++++++++++++------------------------ website/docs/index.md | 2 +- 4 files changed, 200 insertions(+), 231 deletions(-) diff --git a/README.md b/README.md index 8f5249ac..30cb84f5 100644 --- a/README.md +++ b/README.md @@ -3,8 +3,7 @@ Echo is a fast HTTP router (zero memory allocation) and micro web framework in G ## Features -- Fast :rocket: HTTP router which smartly resolves conflicting routes. -- Fast router which smartly resolves conflicting routes. +- Fast :rocket: HTTP router which smartly prioritize routes. - Extensible middleware/handler, supports: - Middleware - `func(*echo.Context)` diff --git a/router.go b/router.go index dca7935c..332f69a5 100644 --- a/router.go +++ b/router.go @@ -204,8 +204,14 @@ func lcp(a, b string) (i int) { func (r *router) Find(method, path string, ctx *Context) (h HandlerFunc, echo *Echo) { cn := r.trees[method] // Current node as root search := path - c := new(node) // Child node - n := 0 // Param counter + + var ( + c *node // Child node + n int // Param counter + nt ntype // Next type + nn *node // Next node + ns string // Next search + ) // TODO: Check empty path??? @@ -219,14 +225,28 @@ func (r *router) Find(method, path string, ctx *Context) (h HandlerFunc, echo *E return } - pl := len(cn.prefix) - l := lcp(search, cn.prefix) + pl := 0 // Prefix length + l := 0 // LCP length + + if cn.label != ':' { + pl = len(cn.prefix) + l = lcp(search, cn.prefix) + } if l == pl { // Continue search search = search[l:] - } else if l < pl && cn.label != ':' { - goto Up + } else { + cn = nn + search = ns + if nt == ptype { + goto Param + } else if nt == mtype { + goto MatchAny + } else { + // Not found + return + } } if search == "" { @@ -241,6 +261,12 @@ func (r *router) Find(method, path string, ctx *Context) (h HandlerFunc, echo *E // Static node c = cn.findSchild(search[0]) if c != nil { + // Save next + if cn.label == '/' { + nt = ptype + nn = cn + ns = search + } cn = c continue } @@ -249,6 +275,12 @@ func (r *router) Find(method, path string, ctx *Context) (h HandlerFunc, echo *E Param: c = cn.findPchild() if c != nil { + // Save next + if cn.label == '/' { + nt = mtype + nn = cn + ns = search + } cn = c i, l := 0, len(search) for ; i < l && search[i] != '/'; i++ { @@ -266,22 +298,10 @@ func (r *router) Find(method, path string, ctx *Context) (h HandlerFunc, echo *E cn = c ctx.pvalues[n] = search search = "" // End search - continue - } - - Up: - tn := cn // Save current node - cn = cn.parent - if cn == nil { + } else { // Not found return } - // Search upwards - if l == pl { - // Reset search - search = tn.prefix + search - } - goto Param } } diff --git a/router_test.go b/router_test.go index 1736c87e..a45a3a2d 100644 --- a/router_test.go +++ b/router_test.go @@ -14,7 +14,7 @@ type route struct { } var ( - context = &Context{pvalues: make([]string, 5)} + context = NewContext(nil, nil, New()) api = []route{ // OAuth Authorizations {"GET", "/authorizations"}, @@ -289,11 +289,12 @@ func TestRouterStatic(t *testing.T) { }, nil) h, _ := r.Find(GET, path, context) if h == nil { - t.Fatal("handler not found") - } - h(nil) - if b.String() != path { - t.Errorf("buffer should %s", path) + t.Error("handler not found") + } else { + h(nil) + if b.String() != path { + t.Errorf("buffer should %s", path) + } } } @@ -304,10 +305,11 @@ func TestRouterParam(t *testing.T) { }, nil) h, _ := r.Find(GET, "/users/1", context) if h == nil { - t.Fatal("handler not found") - } - if context.pvalues[0] != "1" { - t.Error("param id should be 1") + t.Error("handler not found") + } else { + if context.P(0) != "1" { + t.Error("param id should be 1") + } } } @@ -319,13 +321,14 @@ func TestRouterTwoParam(t *testing.T) { h, _ := r.Find(GET, "/users/1/files/1", context) if h == nil { - t.Fatal("handler not found") - } - if context.pvalues[0] != "1" { - t.Error("param uid should be 1") - } - if context.pvalues[1] != "1" { - t.Error("param fid should be 1") + t.Error("handler not found") + } else { + if context.P(0) != "1" { + t.Error("param uid should be 1") + } + if context.P(1) != "1" { + t.Error("param fid should be 1") + } } h, _ = r.Find(GET, "/users/1", context) @@ -342,18 +345,20 @@ func TestRouterMatchAny(t *testing.T) { h, _ := r.Find(GET, "/users/", context) if h == nil { - t.Fatal("handler not found") - } - if context.pvalues[0] != "" { - t.Error("value should be empty") + t.Error("should match empty value") + } else { + if context.P(0) != "" { + t.Error("value should be empty") + } } h, _ = r.Find(GET, "/users/joe", context) if h == nil { - t.Fatal("handler not found") - } - if context.pvalues[0] != "joe" { - t.Error("value should be joe") + t.Error("should match non-empty value") + } else { + if context.P(0) != "joe" { + t.Error("value should be joe") + } } } @@ -364,16 +369,17 @@ func TestRouterMicroParam(t *testing.T) { }, nil) h, _ := r.Find(GET, "/1/2/3", context) if h == nil { - t.Fatal("handler not found") - } - if context.pvalues[0] != "1" { - t.Error("param a should be 1") - } - if context.pvalues[1] != "2" { - t.Error("param b should be 2") - } - if context.pvalues[2] != "3" { - t.Error("param c should be 3") + t.Error("handler not found") + } else { + if context.P(0) != "1" { + t.Error("param a should be 1") + } + if context.P(1) != "2" { + t.Error("param b should be 2") + } + if context.P(2) != "3" { + t.Error("param c should be 3") + } } } @@ -393,140 +399,143 @@ func TestRouterMultiRoute(t *testing.T) { // Route > /users h, _ := r.Find(GET, "/users", context) if h == nil { - t.Fatal("handler not found") - } - h(nil) - if b.String() != "/users" { - t.Errorf("buffer should be /users") + t.Error("handler not found") + } else { + h(nil) + if b.String() != "/users" { + t.Errorf("buffer should be /users") + } } - // Route > /users/:id > /users/1 + // Route > /users/:id h, _ = r.Find(GET, "/users/1", context) if h == nil { - t.Fatal("handler not found") - } - if context.pvalues[0] != "1" { - t.Error("param id should be 1") + t.Error("handler not found") + } else { + if context.P(0) != "1" { + t.Error("param id should be 1") + } } // Route > /user h, _ = r.Find(GET, "/user", context) if h != nil { - t.Fatal("handler should be nil") + t.Error("handler should be nil") } } -func TestRouterConflictingRoute(t *testing.T) { +func TestRouterPriority(t *testing.T) { r := New().Router - b := new(bytes.Buffer) // Routes - r.Add(GET, "/users", func(*Context) *HTTPError { - b.WriteString("/users") + r.Add(GET, "/users", func(c *Context) *HTTPError { + c.Set("a", 1) return nil }, nil) - r.Add(GET, "/users/new", func(*Context) *HTTPError { - b.Reset() - b.WriteString("/users/new") + r.Add(GET, "/users/new", func(c *Context) *HTTPError { + c.Set("b", 2) return nil }, nil) r.Add(GET, "/users/:id", func(c *Context) *HTTPError { + c.Set("c", 3) return nil }, nil) - r.Add(GET, "/users/new/moon", func(*Context) *HTTPError { - b.Reset() - b.WriteString("/users/new/moon") + r.Add(GET, "/users/dew", func(c *Context) *HTTPError { + c.Set("d", 4) return nil }, nil) - r.Add(GET, "/users/new/:id", func(*Context) *HTTPError { + r.Add(GET, "/users/:id/files", func(c *Context) *HTTPError { + c.Set("e", 5) + return nil + }, nil) + r.Add(GET, "/users/newsee", func(c *Context) *HTTPError { + c.Set("f", 6) + return nil + }, nil) + r.Add(GET, "/users/*", func(c *Context) *HTTPError { + c.Set("g", 7) return nil }, nil) // Route > /users h, _ := r.Find(GET, "/users", context) if h == nil { - t.Fatal("handler not found") + t.Error("handler not found") + } else { + h(context) + if context.Get("a") != 1 { + t.Error("a should map to 1") + } } // Route > /users/new h, _ = r.Find(GET, "/users/new", context) if h == nil { - t.Fatal("handler not found") - } - h(nil) - if b.String() != "/users/new" { - t.Error("buffer should be /users/new") + t.Error("handler not found") + } else { + h(context) + if context.Get("b") != 2 { + t.Error("b should map to 2") + } } - // Route > /users/:id > /users/1 + // Route > /users/:id h, _ = r.Find(GET, "/users/1", context) if h == nil { - t.Fatal("handler not found") - } - if context.pvalues[0] != "1" { - t.Error("param id should be 1") + t.Error("handler not found") + } else { + h(context) + if context.Get("c") != 3 { + t.Error("c should map to 3") + } } - // Route > /users/:id > /users/nil - h, _ = r.Find(GET, "/users/nil", context) + // Route > /users/dew + h, _ = r.Find(GET, "/users/dew", context) if h == nil { - t.Fatal("handler not found") - } - if context.pvalues[0] != "nil" { - t.Error("param id should be nil") + t.Error("handler not found") + } else { + h(context) + if context.Get("d") != 4 { + t.Error("d should map to 4") + } } - // Route > /users/:id > /users/news + // Route > /users/:id/files + h, _ = r.Find(GET, "/users/1/files", context) + if h == nil { + t.Error("handler not found") + } else { + h(context) + if context.Get("e") != 5 { + t.Error("e should map to 5") + } + } + + // Route > /users/:id h, _ = r.Find(GET, "/users/news", context) if h == nil { - t.Fatal("handler not found") - } - if context.pvalues[0] != "news" { - t.Error("param id should be news") + t.Error("handler not found") + } else { + h(context) + if context.Get("c") != 3 { + t.Error("c should map to 3") + } } - //----------- - // Two level - //----------- - - // Route > /users/new/moon > /users/new/moon - h, _ = r.Find(GET, "/users/new/moon", context) + // Route > /users/* + h, _ = r.Find(GET, "/users/joe/books", context) if h == nil { - t.Fatal("handler not found") - } - h(nil) - if b.String() != "/users/new/moon" { - t.Error("buffer should be /users/new/moon") - } - - // Route > /users/new/:id > /users/new/1 - h, _ = r.Find(GET, "/users/new/1", context) - if h == nil { - t.Fatal("handler not found") - } - if context.pvalues[0] != "1" { - t.Error("param id should be 1") - } - - // Route > /users/new/:id > /users/new/me - h, _ = r.Find(GET, "/users/new/me", context) - if h == nil { - t.Fatal("handler not found") - } - if context.pvalues[0] != "me" { - t.Error("param id should be me") - } - - // Route > /users/new/:id > /users/new/moons - h, _ = r.Find(GET, "/users/new/moons", context) - if h == nil { - t.Fatal("handler not found") - } - if context.pvalues[0] != "moons" { - t.Error("param id should be moons") + t.Error("handler not found") + } else { + h(context) + if context.Get("g") != 7 { + t.Error("g should map to 7") + } } } + func TestRouterParamNames(t *testing.T) { r := New().Router b := new(bytes.Buffer) @@ -546,41 +555,44 @@ func TestRouterParamNames(t *testing.T) { // Route > /users h, _ := r.Find(GET, "/users", context) if h == nil { - t.Fatal("handler not found") - } - h(nil) - if b.String() != "/users" { - t.Errorf("buffer should be /users") + t.Error("handler not found") + } else { + h(context) + if b.String() != "/users" { + t.Errorf("buffer should be /users") + } } - // Route > /users/:id > /users/1 + // Route > /users/:id h, _ = r.Find(GET, "/users/1", context) if h == nil { - t.Fatal("handler not found") - } - if context.pnames[0] != "id" { - t.Error("param name should be id") - } - if context.pvalues[0] != "1" { - t.Error("param id should be 1") + t.Error("handler not found") + } else { + if context.pnames[0] != "id" { + t.Error("param name should be id") + } + if context.P(0) != "1" { + t.Error("param id should be 1") + } } - // Route > /users/:uid/files/:fid > /users/1/files/1 + // Route > /users/:uid/files/:fid h, _ = r.Find(GET, "/users/1/files/1", context) if h == nil { - t.Fatal("handler not found") - } - if context.pnames[0] != "uid" { - t.Error("param name should be id") - } - if context.pvalues[0] != "1" { - t.Error("param id should be 1") - } - if context.pnames[1] != "fid" { - t.Error("param name should be id") - } - if context.pvalues[1] != "1" { - t.Error("param id should be 1") + t.Error("handler not found") + } else { + if context.pnames[0] != "uid" { + t.Error("param name should be id") + } + if context.P(0) != "1" { + t.Error("param id should be 1") + } + if context.pnames[1] != "fid" { + t.Error("param name should be id") + } + if context.P(1) != "1" { + t.Error("param id should be 1") + } } } @@ -590,7 +602,7 @@ func TestRouterAPI(t *testing.T) { r.Add(route.method, route.path, func(c *Context) *HTTPError { for i, n := range c.pnames { if n != "" { - if ":"+n != c.pvalues[i] { + if ":"+n != c.P(uint8(i)) { t.Errorf("param not found, method=%s, path=%s", route.method, route.path) } } @@ -600,8 +612,9 @@ func TestRouterAPI(t *testing.T) { h, _ := r.Find(route.method, route.path, context) if h == nil { t.Fatalf("handler not found, method=%s, path=%s", route.method, route.path) + } else { + h(context) } - h(context) } } @@ -622,69 +635,6 @@ func TestRouterServeHTTP(t *testing.T) { r.ServeHTTP(w, req) } -func TestRouterExperiment(t *testing.T) { - r := New().Router - r.Add(GET, "/use", func(*Context) *HTTPError { - return nil - }, nil) - r.Add(GET, "/users/*", func(*Context) *HTTPError { - return nil - }, nil) - r.Add(GET, "/users/", func(*Context) *HTTPError { - return nil - }, nil) - // r.Add(GET, "/use", func(*Context) error { - // return nil - // }, nil) - // r.Add(GET, "/users/*", func(*Context) error { - // return nil - // }, nil) - // r.Add(GET, "/users/", func(*Context) error { - // return nil - // }, nil) - // r.Add(GET, "/users/new/*", func(*Context) error { - // return nil - // }, nil) - // r.Add(GET, "/users/new", func(*Context) error { - // return nil - // }, nil) - // r.Add(GET, "/users/:uid", func(*Context) error { - // return nil - // }, nil) - // r.Add(GET, "/users/new/:id", func(*Context) error { - // return nil - // }, nil) - // r.Add(GET, "/users/wen", func(*Context) error { - // return nil - // }, nil) - // r.Add(GET, "/users/:uid/files/:fid", func(*Context) error { - // return nil - // }, nil) - - r.Add(GET, "/users/new", func(*Context) *HTTPError { - return nil - }, nil) - r.Add(GET, "/users/:uid", func(*Context) *HTTPError { - return nil - }, nil) - r.Add(GET, "/users/new/:id", func(*Context) *HTTPError { - return nil - }, nil) - r.Add(GET, "/users/wen", func(*Context) *HTTPError { - return nil - }, nil) - r.Add(GET, "/users/:uid/files/:fid", func(*Context) *HTTPError { - return nil - }, nil) - - r.trees[GET].printTree("", true) - - h, _ := r.Find(GET, "/users/new", context) - if h == nil { - t.Fatal("handler not found") - } -} - func (n *node) printTree(pfx string, tail bool) { p := prefix(tail, pfx, "└── ", "├── ") fmt.Printf("%s%s, %p: type=%d, parent=%p, handler=%v\n", p, n.prefix, n, n.typ, n.parent, n.handler) diff --git a/website/docs/index.md b/website/docs/index.md index 5c89770a..203648ab 100644 --- a/website/docs/index.md +++ b/website/docs/index.md @@ -10,7 +10,7 @@ Echo is a fast HTTP router (zero memory allocation) and micro web framework in G ## Features -- Fast HTTP router which smartly resolves conflicting routes. +- Fast HTTP router which smartly prioritize routes. - Extensible middleware/handler, supports: - Middleware - `func(*echo.Context)`