From ec75c36820198aeeb936e66f267146dc70545306 Mon Sep 17 00:00:00 2001 From: dinever Date: Wed, 20 Apr 2016 17:34:54 -0400 Subject: [PATCH 01/15] [feat] Proof of concept: prefix tree router --- router.go | 166 ++++++++++++----------- router_test.go | 353 +++++++++++++++++++++++++++++++++++++++---------- tree.go | 238 +++++++++++++++++++++++++++++++++ 3 files changed, 612 insertions(+), 145 deletions(-) create mode 100644 tree.go diff --git a/router.go b/router.go index c39c289..1488fab 100644 --- a/router.go +++ b/router.go @@ -1,103 +1,113 @@ -package Golf +package golf import ( - "regexp" - "strings" + "fmt" + "net/http" + "golang.org/x/net/context" ) -const ( - routerMethodGet = "GET" - routerMethodPost = "POST" - routerMethodPut = "PUT" - routerMethodDelete = "DELETE" -) +type Handler func(c context.Context, w http.ResponseWriter, r *http.Request) -type router struct { - routeSlice []*route +type Router struct { + Trees map[string]*Node } -// Handler is the type of the handler function that Golf accepts. -type Handler func(ctx *Context) - -// ErrorHandlerType is the type of the function that handles error in Golf. -type ErrorHandlerType func(ctx *Context, data ...map[string]interface{}) - -// ErrorHandler is the type of the error handler function that Golf accepts. -type ErrorHandler func(ctx *Context, e error) - -type route struct { - method string - pattern string - regex *regexp.Regexp - params []string - handler Handler +func NewRouter() *Router { + return &Router{Trees: make(map[string]*Node)} } -func newRouter() *router { - r := new(router) - r.routeSlice = make([]*route, 0) - return r -} +func splitURLpath(path string) (parts []string, names map[string]int) { -func newRoute(method string, pattern string, handler Handler) *route { - route := new(route) - route.pattern = pattern - route.params = make([]string, 0) - route.regex, route.params = route.parseURL(pattern) - route.method = method - route.handler = handler - return route -} + var ( + nameidx int = -1 + partidx int + paramCounter int + ) -func (router *router) get(pattern string, handler Handler) { - route := newRoute(routerMethodGet, pattern, handler) - router.registerRoute(route, handler) -} + for i := 0; i < len(path); i++ { + + if names == nil { + names = make(map[string]int) + } + // recording name + if nameidx != -1 { + //found / + if path[i] == '/' { + names[path[nameidx:i]] = paramCounter + paramCounter++ + + nameidx = -1 // switch to normal recording + partidx = i + } + } else { + if path[i] == ':' || path[i] == '*' { + if path[i-1] != '/' { + panic(fmt.Errorf("Inválid parameter : or * comes anwais after / - %q", path)) + } + nameidx = i + 1 + if partidx != i { + parts = append(parts, path[partidx:i]) + } + parts = append(parts, path[i:nameidx]) + } + } + } -func (router *router) post(pattern string, handler Handler) { - route := newRoute(routerMethodPost, pattern, handler) - router.registerRoute(route, handler) + if nameidx != -1 { + names[path[nameidx:]] = paramCounter + paramCounter++ + } else if partidx < len(path) { + parts = append(parts, path[partidx:]) + } + return } -func (router *router) put(pattern string, handler Handler) { - route := newRoute(routerMethodPut, pattern, handler) - router.registerRoute(route, handler) +func (router *Router) Finalize() { + for _, _node := range router.Trees { + _node.finalize() + } } -func (router *router) delete(pattern string, handler Handler) { - route := newRoute(routerMethodDelete, pattern, handler) - router.registerRoute(route, handler) +func (router *Router) FindRoute(method string, path string) (Handler, Parameter) { + node := router.Trees[method] + if node == nil { + return nil, Parameter{} + } + matchedNode, wildcard := node.findRoute(path) + if matchedNode != nil { + return matchedNode.handler, Parameter{Node: matchedNode, path: path, wildcard: wildcard} + } + return nil, Parameter{} } -func (router *router) registerRoute(route *route, handler Handler) { - router.routeSlice = append(router.routeSlice, route) +func (router *Router) AddRoute(method string, path string, handler Handler) { + var ( + rootNode *Node + ok bool + ) + parts, names := splitURLpath(path) + if rootNode, ok = router.Trees[method]; !ok { + rootNode = &Node{} + router.Trees[method] = rootNode + } + rootNode.addRoute(parts, names, handler) + rootNode.optimizeRoutes() } -func (router *router) match(url string, method string) (params map[string]string, handler Handler) { - params = make(map[string]string) - for _, route := range router.routeSlice { - if method == route.method && route.regex.MatchString(url) { - subMatch := route.regex.FindStringSubmatch(url) - for i, param := range route.params { - params[param] = subMatch[i+1] - } - handler = route.handler - return params, handler - } +func (router *Router) String() string { + var lines string + for method, _node := range router.Trees { + lines += method + " " + _node.String() } - return nil, nil + return lines } -// Parse the URL to a regexp and a map of parameters -func (route *route) parseURL(pattern string) (regex *regexp.Regexp, params []string) { - params = make([]string, 0) - segments := strings.Split(pattern, "/") - for i, segment := range segments { - if strings.HasPrefix(segment, ":") { - segments[i] = `([\w-%]+)` - params = append(params, strings.TrimPrefix(segment, ":")) - } +func (router *Router) ServeHTTP(w http.ResponseWriter, r *http.Request) { + handler, variables := router.FindRoute(r.Method, r.URL.Path) + + if handler != nil { + handler(w, r, variables) + } else { + http.NotFound(w, r) } - regex, _ = regexp.Compile("^" + strings.Join(segments, "/") + "$") - return regex, params } diff --git a/router_test.go b/router_test.go index 486b91c..396f2b1 100644 --- a/router_test.go +++ b/router_test.go @@ -1,82 +1,301 @@ -package Golf +package golf import ( - "reflect" + "fmt" "testing" + "golang.org/x/net/context" + "net/http" ) -func handler(ctx *Context) {} - -func TestParsePatternWithOneParam(t *testing.T) { - cases := []struct { - method, in, regex, param string - }{ - {routerMethodGet, "/:id/", `^/([\w-%]+)/$`, "id"}, - {routerMethodPost, "/:id/", `^/([\w-%]+)/$`, "id"}, - {routerMethodPut, "/:id/", `^/([\w-%]+)/$`, "id"}, - {routerMethodDelete, "/:id/", `^/([\w-%]+)/$`, "id"}, +func assert_equals_string(one, two []string) bool { + if len(one) != len(two) { + return false } - - for _, c := range cases { - route := newRoute(c.method, c.in, handler) - if route.regex.String() != c.regex { - t.Errorf("regex of %q == %q, want %q", c.in, route.regex.String(), c.regex) - } - if len(route.params) != 1 { - t.Errorf("%q is supposed to have 1 parameter", c.in) - } - if route.params[0] != "id" { - t.Errorf("params[0] == %q, want %q", c.in, c.param) + for i := 0; i < len(one); i++ { + if one[i] != two[i] { + return false } } + return true } -func TestParsePatternWithThreeParam(t *testing.T) { - cases := []struct { - in, regex string - params []string - }{ - { - "/:year/:month/:day/", - `^/([\w-%]+)/([\w-%]+)/([\w-%]+)/$`, - []string{"year", "month", "day"}, - }, - } +type route struct { + method string + path string +} - for _, c := range cases { - route := newRoute(routerMethodGet, c.in, handler) - if route.regex.String() != c.regex { - t.Errorf("regex == %q, want %q", c.in, route.regex.String()) - } - if !reflect.DeepEqual(route.params, c.params) { - t.Errorf("parameters not match: %v != %v", route.params, c.params) - } - } +var githubAPI = []route{ + // OAuth Authorizations + {"GET", "/authorizations"}, + {"GET", "/authorizations/:id"}, + {"POST", "/authorizations"}, + //{"PUT", "/authorizations/clients/:client_id"}, + //{"PATCH", "/authorizations/:id"}, + {"DELETE", "/authorizations/:id"}, + {"GET", "/applications/:client_id/tokens/:access_token"}, + {"DELETE", "/applications/:client_id/tokens"}, + {"DELETE", "/applications/:client_id/tokens/:access_token"}, + + // Activity + {"GET", "/events"}, + {"GET", "/repos/:owner/:repo/events"}, + {"GET", "/networks/:owner/:repo/events"}, + {"GET", "/orgs/:org/events"}, + {"GET", "/users/:user/received_events"}, + {"GET", "/users/:user/received_events/public"}, + {"GET", "/users/:user/events"}, + {"GET", "/users/:user/events/public"}, + {"GET", "/users/:user/events/orgs/:org"}, + {"GET", "/feeds"}, + {"GET", "/notifications"}, + {"GET", "/repos/:owner/:repo/notifications"}, + {"PUT", "/notifications"}, + {"PUT", "/repos/:owner/:repo/notifications"}, + {"GET", "/notifications/threads/:id"}, + //{"PATCH", "/notifications/threads/:id"}, + {"GET", "/notifications/threads/:id/subscription"}, + {"PUT", "/notifications/threads/:id/subscription"}, + {"DELETE", "/notifications/threads/:id/subscription"}, + {"GET", "/repos/:owner/:repo/stargazers"}, + {"GET", "/users/:user/starred"}, + {"GET", "/user/starred"}, + {"GET", "/user/starred/:owner/:repo"}, + {"PUT", "/user/starred/:owner/:repo"}, + {"DELETE", "/user/starred/:owner/:repo"}, + {"GET", "/repos/:owner/:repo/subscribers"}, + {"GET", "/users/:user/subscriptions"}, + {"GET", "/user/subscriptions"}, + {"GET", "/repos/:owner/:repo/subscription"}, + {"PUT", "/repos/:owner/:repo/subscription"}, + {"DELETE", "/repos/:owner/:repo/subscription"}, + {"GET", "/user/subscriptions/:owner/:repo"}, + {"PUT", "/user/subscriptions/:owner/:repo"}, + {"DELETE", "/user/subscriptions/:owner/:repo"}, + + // Gists + {"GET", "/users/:user/gists"}, + {"GET", "/gists"}, + //{"GET", "/gists/public"}, + //{"GET", "/gists/starred"}, + {"GET", "/gists/:id"}, + {"POST", "/gists"}, + //{"PATCH", "/gists/:id"}, + {"PUT", "/gists/:id/star"}, + {"DELETE", "/gists/:id/star"}, + {"GET", "/gists/:id/star"}, + {"POST", "/gists/:id/forks"}, + {"DELETE", "/gists/:id"}, + + // Git Data + {"GET", "/repos/:owner/:repo/git/blobs/:sha"}, + {"POST", "/repos/:owner/:repo/git/blobs"}, + {"GET", "/repos/:owner/:repo/git/commits/:sha"}, + {"POST", "/repos/:owner/:repo/git/commits"}, + //{"GET", "/repos/:owner/:repo/git/refs/*ref"}, + {"GET", "/repos/:owner/:repo/git/refs"}, + {"POST", "/repos/:owner/:repo/git/refs"}, + //{"PATCH", "/repos/:owner/:repo/git/refs/*ref"}, + //{"DELETE", "/repos/:owner/:repo/git/refs/*ref"}, + {"GET", "/repos/:owner/:repo/git/tags/:sha"}, + {"POST", "/repos/:owner/:repo/git/tags"}, + {"GET", "/repos/:owner/:repo/git/trees/:sha"}, + {"POST", "/repos/:owner/:repo/git/trees"}, + + // Issues + {"GET", "/issues"}, + {"GET", "/user/issues"}, + {"GET", "/orgs/:org/issues"}, + {"GET", "/repos/:owner/:repo/issues"}, + {"GET", "/repos/:owner/:repo/issues/:number"}, + {"POST", "/repos/:owner/:repo/issues"}, + //{"PATCH", "/repos/:owner/:repo/issues/:number"}, + {"GET", "/repos/:owner/:repo/assignees"}, + {"GET", "/repos/:owner/:repo/assignees/:assignee"}, + {"GET", "/repos/:owner/:repo/issues/:number/comments"}, + //{"GET", "/repos/:owner/:repo/issues/comments"}, + //{"GET", "/repos/:owner/:repo/issues/comments/:id"}, + {"POST", "/repos/:owner/:repo/issues/:number/comments"}, + //{"PATCH", "/repos/:owner/:repo/issues/comments/:id"}, + //{"DELETE", "/repos/:owner/:repo/issues/comments/:id"}, + {"GET", "/repos/:owner/:repo/issues/:number/events"}, + //{"GET", "/repos/:owner/:repo/issues/events"}, + //{"GET", "/repos/:owner/:repo/issues/events/:id"}, + {"GET", "/repos/:owner/:repo/labels"}, + {"GET", "/repos/:owner/:repo/labels/:name"}, + {"POST", "/repos/:owner/:repo/labels"}, + //{"PATCH", "/repos/:owner/:repo/labels/:name"}, + {"DELETE", "/repos/:owner/:repo/labels/:name"}, + {"GET", "/repos/:owner/:repo/issues/:number/labels"}, + {"POST", "/repos/:owner/:repo/issues/:number/labels"}, + {"DELETE", "/repos/:owner/:repo/issues/:number/labels/:name"}, + {"PUT", "/repos/:owner/:repo/issues/:number/labels"}, + {"DELETE", "/repos/:owner/:repo/issues/:number/labels"}, + {"GET", "/repos/:owner/:repo/milestones/:number/labels"}, + {"GET", "/repos/:owner/:repo/milestones"}, + {"GET", "/repos/:owner/:repo/milestones/:number"}, + {"POST", "/repos/:owner/:repo/milestones"}, + //{"PATCH", "/repos/:owner/:repo/milestones/:number"}, + {"DELETE", "/repos/:owner/:repo/milestones/:number"}, + + // Miscellaneous + {"GET", "/emojis"}, + {"GET", "/gitignore/templates"}, + {"GET", "/gitignore/templates/:name"}, + {"POST", "/markdown"}, + {"POST", "/markdown/raw"}, + {"GET", "/meta"}, + {"GET", "/rate_limit"}, + + // Organizations + {"GET", "/users/:user/orgs"}, + {"GET", "/user/orgs"}, + {"GET", "/orgs/:org"}, + //{"PATCH", "/orgs/:org"}, + {"GET", "/orgs/:org/members"}, + {"GET", "/orgs/:org/members/:user"}, + {"DELETE", "/orgs/:org/members/:user"}, + {"GET", "/orgs/:org/public_members"}, + {"GET", "/orgs/:org/public_members/:user"}, + {"PUT", "/orgs/:org/public_members/:user"}, + {"DELETE", "/orgs/:org/public_members/:user"}, + {"GET", "/orgs/:org/teams"}, + {"GET", "/teams/:id"}, + {"POST", "/orgs/:org/teams"}, + //{"PATCH", "/teams/:id"}, + {"DELETE", "/teams/:id"}, + {"GET", "/teams/:id/members"}, + {"GET", "/teams/:id/members/:user"}, + {"PUT", "/teams/:id/members/:user"}, + {"DELETE", "/teams/:id/members/:user"}, + {"GET", "/teams/:id/repos"}, + {"GET", "/teams/:id/repos/:owner/:repo"}, + {"PUT", "/teams/:id/repos/:owner/:repo"}, + {"DELETE", "/teams/:id/repos/:owner/:repo"}, + {"GET", "/user/teams"}, + + // Pull Requests + {"GET", "/repos/:owner/:repo/pulls"}, + {"GET", "/repos/:owner/:repo/pulls/:number"}, + {"POST", "/repos/:owner/:repo/pulls"}, + //{"PATCH", "/repos/:owner/:repo/pulls/:number"}, + {"GET", "/repos/:owner/:repo/pulls/:number/commits"}, + {"GET", "/repos/:owner/:repo/pulls/:number/files"}, + {"GET", "/repos/:owner/:repo/pulls/:number/merge"}, + {"PUT", "/repos/:owner/:repo/pulls/:number/merge"}, + {"GET", "/repos/:owner/:repo/pulls/:number/comments"}, + //{"GET", "/repos/:owner/:repo/pulls/comments"}, + //{"GET", "/repos/:owner/:repo/pulls/comments/:number"}, + {"PUT", "/repos/:owner/:repo/pulls/:number/comments"}, + //{"PATCH", "/repos/:owner/:repo/pulls/comments/:number"}, + //{"DELETE", "/repos/:owner/:repo/pulls/comments/:number"}, + + // Repositories + {"GET", "/user/repos"}, + {"GET", "/users/:user/repos"}, + {"GET", "/orgs/:org/repos"}, + {"GET", "/repositories"}, + {"POST", "/user/repos"}, + {"POST", "/orgs/:org/repos"}, + {"GET", "/repos/:owner/:repo"}, + //{"PATCH", "/repos/:owner/:repo"}, + {"GET", "/repos/:owner/:repo/contributors"}, + {"GET", "/repos/:owner/:repo/languages"}, + {"GET", "/repos/:owner/:repo/teams"}, + {"GET", "/repos/:owner/:repo/tags"}, + {"GET", "/repos/:owner/:repo/branches"}, + {"GET", "/repos/:owner/:repo/branches/:branch"}, + {"DELETE", "/repos/:owner/:repo"}, + {"GET", "/repos/:owner/:repo/collaborators"}, + {"GET", "/repos/:owner/:repo/collaborators/:user"}, + {"PUT", "/repos/:owner/:repo/collaborators/:user"}, + {"DELETE", "/repos/:owner/:repo/collaborators/:user"}, + {"GET", "/repos/:owner/:repo/comments"}, + {"GET", "/repos/:owner/:repo/commits/:sha/comments"}, + {"POST", "/repos/:owner/:repo/commits/:sha/comments"}, + {"GET", "/repos/:owner/:repo/comments/:id"}, + //{"PATCH", "/repos/:owner/:repo/comments/:id"}, + {"DELETE", "/repos/:owner/:repo/comments/:id"}, + {"GET", "/repos/:owner/:repo/commits"}, + {"GET", "/repos/:owner/:repo/commits/:sha"}, + {"GET", "/repos/:owner/:repo/readme"}, + //{"GET", "/repos/:owner/:repo/contents/*path"}, + //{"PUT", "/repos/:owner/:repo/contents/*path"}, + //{"DELETE", "/repos/:owner/:repo/contents/*path"}, + //{"GET", "/repos/:owner/:repo/:archive_format/:ref"}, + {"GET", "/repos/:owner/:repo/keys"}, + {"GET", "/repos/:owner/:repo/keys/:id"}, + {"POST", "/repos/:owner/:repo/keys"}, + //{"PATCH", "/repos/:owner/:repo/keys/:id"}, + {"DELETE", "/repos/:owner/:repo/keys/:id"}, + {"GET", "/repos/:owner/:repo/downloads"}, + {"GET", "/repos/:owner/:repo/downloads/:id"}, + {"DELETE", "/repos/:owner/:repo/downloads/:id"}, + {"GET", "/repos/:owner/:repo/forks"}, + {"POST", "/repos/:owner/:repo/forks"}, + {"GET", "/repos/:owner/:repo/hooks"}, + {"GET", "/repos/:owner/:repo/hooks/:id"}, + {"POST", "/repos/:owner/:repo/hooks"}, + //{"PATCH", "/repos/:owner/:repo/hooks/:id"}, + {"POST", "/repos/:owner/:repo/hooks/:id/tests"}, + {"DELETE", "/repos/:owner/:repo/hooks/:id"}, + {"POST", "/repos/:owner/:repo/merges"}, + {"GET", "/repos/:owner/:repo/releases"}, + {"GET", "/repos/:owner/:repo/releases/:id"}, + {"POST", "/repos/:owner/:repo/releases"}, + //{"PATCH", "/repos/:owner/:repo/releases/:id"}, + {"DELETE", "/repos/:owner/:repo/releases/:id"}, + {"GET", "/repos/:owner/:repo/releases/:id/assets"}, + {"GET", "/repos/:owner/:repo/stats/contributors"}, + {"GET", "/repos/:owner/:repo/stats/commit_activity"}, + {"GET", "/repos/:owner/:repo/stats/code_frequency"}, + {"GET", "/repos/:owner/:repo/stats/participation"}, + {"GET", "/repos/:owner/:repo/stats/punch_card"}, + {"GET", "/repos/:owner/:repo/statuses/:ref"}, + {"POST", "/repos/:owner/:repo/statuses/:ref"}, + + // Search + {"GET", "/search/repositories"}, + {"GET", "/search/code"}, + {"GET", "/search/issues"}, + {"GET", "/search/users"}, + {"GET", "/legacy/issues/search/:owner/:repository/:state/:keyword"}, + {"GET", "/legacy/repos/search/:keyword"}, + {"GET", "/legacy/user/search/:keyword"}, + {"GET", "/legacy/user/email/:email"}, + + // Users + {"GET", "/users/:user"}, + {"GET", "/user"}, + //{"PATCH", "/user"}, + {"GET", "/users"}, + {"GET", "/user/emails"}, + {"POST", "/user/emails"}, + {"DELETE", "/user/emails"}, + {"GET", "/users/:user/followers"}, + {"GET", "/user/followers"}, + {"GET", "/users/:user/following"}, + {"GET", "/user/following"}, + {"GET", "/user/following/:user"}, + {"GET", "/users/:user/following/:target_user"}, + {"PUT", "/user/following/:user"}, + {"DELETE", "/user/following/:user"}, + {"GET", "/users/:user/keys"}, + {"GET", "/user/keys"}, + {"GET", "/user/keys/:id"}, + {"POST", "/user/keys"}, + //{"PATCH", "/user/keys/:id"}, + {"DELETE", "/user/keys/:id"}, } -func TestRouterMatch(t *testing.T) { - router := newRouter() - cases := []struct { - pattern string - url string - params map[string]string - }{ - { - "/:year/:month/:day/", - "/2015/11/15/", - map[string]string{"year": "2015", "month": "11", "day": "15"}, - }, - { - "/user/:id/", - "/user/foobar/", - map[string]string{"id": "foobar"}, - }, - } - for _, c := range cases { - router.get(c.pattern, handler) - params, _ := router.match(c.url, routerMethodGet) - if !reflect.DeepEqual(params, c.params) { - t.Errorf("parameters not match: %v != %v", params, c.params) - } + +func handler(c context.Context, w http.ResponseWriter, r *http.Request) { + fmt.Fprintf(w, "Home") +} + + +func TestRouter(t *testing.T) { + router := NewRouter() + for _, route := range githubAPI { + router.AddRoute(route.method, route.path, handler) } } diff --git a/tree.go b/tree.go new file mode 100644 index 0000000..8e8c881 --- /dev/null +++ b/tree.go @@ -0,0 +1,238 @@ +package golf + +import ( + "sort" + "strings" +) + +type Node struct { + text string + names map[string]int + handler Handler + + parent *Node + wildcard *Node + colon *Node + + nodes nodes + start byte + max byte + indices []uint8 +} + +type nodes []*Node + +func (s nodes) Len() int { + return len(s) +} + +func (s nodes) Swap(i, j int) { + s[i], s[j] = s[j], s[i] +} + +func (s nodes) Less(i, j int) bool { + return s[i].text[0] < s[j].text[0] +} + +func (n *Node) matchNode(path string) (*Node, int8, int) { + + if path == "*" { + if n.wildcard == nil { + n.wildcard = &Node{text: "*"} + } + return n.wildcard, 0, 0 + } + + if path == ":" { + if n.colon == nil { + n.colon = &Node{text: ":"} + } + return n.colon, 0, 0 + } + + for i, node := range n.nodes { + if node.text[0] == path[0] { + + maxLength := len(node.text) + pathLength := len(path) + var pathCompare int8 + + if pathLength > maxLength { + pathCompare = 1 + } else if pathLength < maxLength { + maxLength = pathLength + pathCompare = -1 + } + + for j := 0; j < maxLength; j++ { + if path[j] != node.text[j] { + ccNode := &Node{text: path[0:j], nodes: nodes{node, &Node{text: path[j:]}}} + node.text = node.text[j:] + n.nodes[i] = ccNode + return ccNode.nodes[1], 0, i + } + } + + return node, pathCompare, i + } + } + + return nil, 0, 0 +} + +func (n *Node) addRoute(parts []string, names map[string]int, handler Handler) { + + var ( + tmpNode *Node + currentNode *Node + loop = true + ) + + currentNode, result, i := n.matchNode(parts[0]) + + for loop == true { + if currentNode == nil { + currentNode = &Node{text: parts[0]} + n.nodes = append(n.nodes, currentNode) + } else if result == 1 { + // + parts[0] = parts[0][len(currentNode.text):] + tmpNode, result, i = currentNode.matchNode(parts[0]) + n = currentNode + currentNode = tmpNode + continue + } else if result == -1 { + tmpNode := &Node{text: parts[0]} + currentNode.text = currentNode.text[len(tmpNode.text):] + tmpNode.nodes = nodes{currentNode} + n.nodes[i] = tmpNode + currentNode = tmpNode + } + break + } + + if len(parts) == 1 { + currentNode.handler = handler + currentNode.names = names + return + } + + currentNode.addRoute(parts[1:], names, handler) +} + +func (n *Node) findRoute(urlPath string) (*Node, int) { + + urlByte := urlPath[0] + pathLen := len(urlPath) + + if urlByte >= n.start && urlByte <= n.max { + if i := n.indices[urlByte-n.start]; i != 0 { + matched := n.nodes[i-1] + nodeLen := len(matched.text) + if nodeLen < pathLen { + if matched.text == urlPath[:nodeLen] { + if matched, wildcard := matched.findRoute(urlPath[nodeLen:]); matched != nil { + return matched, wildcard + } + } + } else if matched.text == urlPath { + if matched.handler == nil && matched.wildcard != nil { + return matched.wildcard, 0 + } + return matched, 0 + } + } + } + + if n.colon != nil && pathLen != 0 { + i := strings.IndexByte(urlPath, '/') + if i > 0 { + if cNode, wildcard := n.colon.findRoute(urlPath[i:]); cNode != nil { + return cNode, wildcard + } + } else if n.colon.handler != nil { + return n.colon, 0 + } + } + + if n.wildcard != nil { + return n.wildcard, pathLen + } + + return nil, 0 +} + +func (n *Node) optimizeRoutes() { + + if len(n.nodes) > 0 { + sort.Sort(n.nodes) + for i := 0; i < len(n.indices); i++ { + n.indices[i] = 0 + } + + n.start = n.nodes[0].text[0] + n.max = n.nodes[len(n.nodes)-1].text[0] + + for i := 0; i < len(n.nodes); i++ { + cNode := n.nodes[i] + cNode.parent = n + + cByte := int(cNode.text[0] - n.start) + if cByte >= len(n.indices) { + n.indices = append(n.indices, make([]uint8, cByte+1-len(n.indices))...) + } + n.indices[cByte] = uint8(i + 1) + cNode.optimizeRoutes() + } + } + + if n.colon != nil { + n.colon.parent = n + n.colon.optimizeRoutes() + } + + if n.wildcard != nil { + n.wildcard.parent = n + n.wildcard.optimizeRoutes() + } +} + +func (_node *Node) finalize() { + if len(_node.nodes) > 0 { + for i := 0; i < len(_node.nodes); i++ { + _node.nodes[i].finalize() + } + } + if _node.colon != nil { + _node.colon.finalize() + } + if _node.wildcard != nil { + _node.wildcard.finalize() + } + *_node = Node{} +} + +func (_node *Node) string(col int) string { + var str = "\n" + strings.Repeat(" ", col) + _node.text + " -> " + col += len(_node.text) + 4 + for i := 0; i < len(_node.indices); i++ { + if j := _node.indices[i]; j != 0 { + str += _node.nodes[j-1].string(col) + } + } + if _node.colon != nil { + str += _node.colon.string(col) + } + if _node.wildcard != nil { + str += _node.wildcard.string(col) + } + return str +} + +func (_node *Node) String() string { + if _node.text == "" { + return _node.string(0) + } + col := len(_node.text) + 4 + return _node.text + " -> " + _node.string(col) +} From e9b8100d54d1b1ecf2e9422f07afe4e90a4af3fc Mon Sep 17 00:00:00 2001 From: dinever Date: Wed, 20 Apr 2016 23:13:16 -0400 Subject: [PATCH 02/15] [refactor] Rename Golf to golf Introduce prefix tree-based router --- README.md | 6 +- app.go | 18 +-- config.go | 2 +- config_test.go | 2 +- context.go | 12 +- context_test.go | 4 +- error.go | 2 +- middleware.go | 2 +- router.go | 96 +++++++++---- router_test.go | 341 ++++++++++------------------------------------- session.go | 2 +- session_test.go | 2 +- template.go | 2 +- template_test.go | 2 +- tree.go | 125 +++++++---------- view.go | 2 +- view_test.go | 2 +- xsrf.go | 2 +- xsrf_test.go | 2 +- 19 files changed, 217 insertions(+), 409 deletions(-) diff --git a/README.md b/README.md index 2183cfe..3bd157d 100644 --- a/README.md +++ b/README.md @@ -20,11 +20,11 @@ package main import "github.com/dinever/golf" -func mainHandler(ctx *Golf.Context) { +func mainHandler(ctx *golf.Context) { ctx.Write("Hello World!") } -func pageHandler(ctx *Golf.Context) { +func pageHandler(ctx *golf.Context) { page, err := ctx.Param("page") if err != nil { ctx.Abort(500) @@ -34,7 +34,7 @@ func pageHandler(ctx *Golf.Context) { } func main() { - app := Golf.New() + app := golf.New() app.Get("/", mainHandler) app.Get("/p/:page/", pageHandler) app.Run(":9000") diff --git a/app.go b/app.go index ddb6f8f..b294a9a 100644 --- a/app.go +++ b/app.go @@ -1,4 +1,4 @@ -package Golf +package golf import ( "net/http" @@ -67,12 +67,12 @@ func (app *Application) handler(ctx *Context) { } } - params, handler := app.router.match(ctx.Request.URL.Path, ctx.Request.Method) - if handler != nil { + handler, params, err := app.router.FindRoute(ctx.Request.Method, ctx.Request.URL.Path) + if err != nil { + app.handleError(ctx, 404) + } else { ctx.Params = params handler(ctx) - } else { - app.handleError(ctx, 404) } ctx.Send() } @@ -112,22 +112,22 @@ func (app *Application) Static(url string, path string) { // Get method is used for registering a Get method route func (app *Application) Get(pattern string, handler Handler) { - app.router.get(pattern, handler) + app.router.AddRoute("GET", pattern, handler) } // Post method is used for registering a Post method route func (app *Application) Post(pattern string, handler Handler) { - app.router.post(pattern, handler) + app.router.AddRoute("POST", pattern, handler) } // Put method is used for registering a Put method route func (app *Application) Put(pattern string, handler Handler) { - app.router.put(pattern, handler) + app.router.AddRoute("PUT", pattern, handler) } // Delete method is used for registering a Delete method route func (app *Application) Delete(pattern string, handler Handler) { - app.router.delete(pattern, handler) + app.router.AddRoute("DELETE", pattern, handler) } // Error method is used for registering an handler for a specified HTTP error code. diff --git a/config.go b/config.go index 80004e8..305506e 100644 --- a/config.go +++ b/config.go @@ -1,4 +1,4 @@ -package Golf +package golf import ( "encoding/json" diff --git a/config_test.go b/config_test.go index eb0302e..c118c05 100644 --- a/config_test.go +++ b/config_test.go @@ -1,4 +1,4 @@ -package Golf +package golf import ( "bytes" diff --git a/context.go b/context.go index b2d2d0d..5192397 100644 --- a/context.go +++ b/context.go @@ -1,12 +1,12 @@ -package Golf +package golf import ( "encoding/hex" "encoding/json" "errors" + "fmt" "net/http" "time" - "fmt" ) // Context is a wrapper of http.Request and http.ResponseWriter. @@ -18,7 +18,7 @@ type Context struct { Response http.ResponseWriter // URL Parameter - Params map[string]string + Params Parameter // HTTP status code StatusCode int @@ -98,10 +98,8 @@ func (ctx *Context) Query(key string, index ...int) (string, error) { // Param method retrieves the parameters from url // If the url is /:id/, then id can be retrieved by calling `ctx.Param(id)` func (ctx *Context) Param(key string) (string, error) { - if val, ok := ctx.Params[key]; ok { - return val, nil - } - return "", errors.New("Parameter not found.") + val, err := ctx.Params.ByName(key) + return val, err } // Redirect method sets the response as a 301 redirection. diff --git a/context_test.go b/context_test.go index c17aeb4..27a1e3c 100644 --- a/context_test.go +++ b/context_test.go @@ -1,7 +1,8 @@ -package Golf +package golf import ( "bufio" + "encoding/hex" "fmt" "io" "net/http" @@ -9,7 +10,6 @@ import ( "reflect" "strings" "testing" - "encoding/hex" ) func makeTestHTTPRequest(body io.Reader, method, url string) *http.Request { diff --git a/error.go b/error.go index 93e61f9..6e40a3f 100644 --- a/error.go +++ b/error.go @@ -1,4 +1,4 @@ -package Golf +package golf import ( "bytes" diff --git a/middleware.go b/middleware.go index 71e0e17..99e8c13 100644 --- a/middleware.go +++ b/middleware.go @@ -1,4 +1,4 @@ -package Golf +package golf import ( "log" diff --git a/router.go b/router.go index 1488fab..0189e0f 100644 --- a/router.go +++ b/router.go @@ -2,24 +2,27 @@ package golf import ( "fmt" - "net/http" - "golang.org/x/net/context" + "strings" ) -type Handler func(c context.Context, w http.ResponseWriter, r *http.Request) +// Handler is the type of the handler function that Golf accepts. +type Handler func(ctx *Context) -type Router struct { - Trees map[string]*Node +// ErrorHandlerType is the type of the function that handles error in Golf. +type ErrorHandlerType func(ctx *Context, data ...map[string]interface{}) + +type router struct { + trees map[string]*Node } -func NewRouter() *Router { - return &Router{Trees: make(map[string]*Node)} +func newRouter() *router { + return &router{trees: make(map[string]*Node)} } func splitURLpath(path string) (parts []string, names map[string]int) { var ( - nameidx int = -1 + nameidx = -1 partidx int paramCounter int ) @@ -62,52 +65,89 @@ func splitURLpath(path string) (parts []string, names map[string]int) { return } -func (router *Router) Finalize() { - for _, _node := range router.Trees { +func (router *router) Finalize() { + for _, _node := range router.trees { _node.finalize() } } -func (router *Router) FindRoute(method string, path string) (Handler, Parameter) { - node := router.Trees[method] +func (router *router) FindRoute(method string, path string) (Handler, Parameter, error) { + node := router.trees[method] if node == nil { - return nil, Parameter{} + return nil, Parameter{}, fmt.Errorf("Can not find route") } - matchedNode, wildcard := node.findRoute(path) - if matchedNode != nil { - return matchedNode.handler, Parameter{Node: matchedNode, path: path, wildcard: wildcard} + matchedNode, err := node.findRoute(path) + if err != nil { + return nil, Parameter{}, err } - return nil, Parameter{} + return matchedNode.handler, Parameter{Node: matchedNode, path: path}, err } -func (router *Router) AddRoute(method string, path string, handler Handler) { +func (router *router) AddRoute(method string, path string, handler Handler) { var ( rootNode *Node ok bool ) parts, names := splitURLpath(path) - if rootNode, ok = router.Trees[method]; !ok { + if rootNode, ok = router.trees[method]; !ok { rootNode = &Node{} - router.Trees[method] = rootNode + router.trees[method] = rootNode } rootNode.addRoute(parts, names, handler) rootNode.optimizeRoutes() } -func (router *Router) String() string { +func (router *router) String() string { var lines string - for method, _node := range router.Trees { + for method, _node := range router.trees { lines += method + " " + _node.String() } return lines } -func (router *Router) ServeHTTP(w http.ResponseWriter, r *http.Request) { - handler, variables := router.FindRoute(r.Method, r.URL.Path) +//Parameter holds the parameters matched in the route +type Parameter struct { + *Node // matched node + path string // url path given + cached map[string]string +} + +//Len returns number arguments matched in the provided URL +func (p *Parameter) Len() int { + return len(p.names) +} - if handler != nil { - handler(w, r, variables) - } else { - http.NotFound(w, r) +//ByName returns the url parameter by name +func (p *Parameter) ByName(name string) (string, error) { + if i, has := p.names[name]; has { + return p.findParam(i) + } + return "", fmt.Errorf("Parameter not found") +} + +//findParam walks up the matched node looking for parameters returns the last parameter +func (p *Parameter) findParam(idx int) (string, error) { + index := len(p.names) - 1 + urlPath := p.path + pathLen := len(p.path) + node := p.Node + + for node != nil { + if node.text[0] == ':' { + ctn := strings.LastIndexByte(urlPath, '/') + if ctn == -1 { + return "", fmt.Errorf("Parameter not found") + } + pathLen = ctn + 1 + if index == idx { + return urlPath[pathLen:], nil + } + index-- + } else { + pathLen -= len(node.text) + } + urlPath = urlPath[0:pathLen] + node = node.parent } + return "", fmt.Errorf("Parameter not found") } diff --git a/router_test.go b/router_test.go index 396f2b1..228b16e 100644 --- a/router_test.go +++ b/router_test.go @@ -1,301 +1,96 @@ package golf import ( - "fmt" "testing" - "golang.org/x/net/context" - "net/http" ) -func assert_equals_string(one, two []string) bool { - if len(one) != len(two) { - return false +func assertStringEqual(t *testing.T, expected, got string) { + if expected != got { + t.Errorf("Expected %v, got %v", expected, got) } - for i := 0; i < len(one); i++ { - if one[i] != two[i] { - return false +} + +func assertSliceEqual(t *testing.T, expected, got []string) { + if len(expected) != len(got) { + t.Errorf("Slice length not equal, expected: %v, got %v", expected, got) + } + for i := 0; i < len(expected); i++ { + if expected[i] != got[i] { + t.Errorf("Slice not equal, expected: %v, got %v", expected, got) } } - return true } type route struct { - method string - path string + method string + path string + testPath string + params map[string]string } var githubAPI = []route{ // OAuth Authorizations - {"GET", "/authorizations"}, - {"GET", "/authorizations/:id"}, - {"POST", "/authorizations"}, - //{"PUT", "/authorizations/clients/:client_id"}, - //{"PATCH", "/authorizations/:id"}, - {"DELETE", "/authorizations/:id"}, - {"GET", "/applications/:client_id/tokens/:access_token"}, - {"DELETE", "/applications/:client_id/tokens"}, - {"DELETE", "/applications/:client_id/tokens/:access_token"}, + {"GET", "/authorizations", "/authorizations", map[string]string{}}, + {"GET", "/authorizations/:id", "/authorizations/12345", map[string]string{"id": "12345"}}, + {"POST", "/authorizations", "/authorizations", map[string]string{}}, + {"DELETE", "/authorizations/:id", "/authorizations/12345", map[string]string{"id": "12345"}}, + {"GET", "/applications/:client_id/tokens/:access_token", "/applications/12345/tokens/67890", map[string]string{"client_id": "12345", "access_token": "67890"}}, + {"DELETE", "/applications/:client_id/tokens", "/applications/12345/tokens", map[string]string{"client_id": "12345"}}, + {"DELETE", "/applications/:client_id/tokens/:access_token", "/applications/12345/tokens/67890", map[string]string{"client_id": "12345", "access_token": "67890"}}, // Activity - {"GET", "/events"}, - {"GET", "/repos/:owner/:repo/events"}, - {"GET", "/networks/:owner/:repo/events"}, - {"GET", "/orgs/:org/events"}, - {"GET", "/users/:user/received_events"}, - {"GET", "/users/:user/received_events/public"}, - {"GET", "/users/:user/events"}, - {"GET", "/users/:user/events/public"}, - {"GET", "/users/:user/events/orgs/:org"}, - {"GET", "/feeds"}, - {"GET", "/notifications"}, - {"GET", "/repos/:owner/:repo/notifications"}, - {"PUT", "/notifications"}, - {"PUT", "/repos/:owner/:repo/notifications"}, - {"GET", "/notifications/threads/:id"}, - //{"PATCH", "/notifications/threads/:id"}, - {"GET", "/notifications/threads/:id/subscription"}, - {"PUT", "/notifications/threads/:id/subscription"}, - {"DELETE", "/notifications/threads/:id/subscription"}, - {"GET", "/repos/:owner/:repo/stargazers"}, - {"GET", "/users/:user/starred"}, - {"GET", "/user/starred"}, - {"GET", "/user/starred/:owner/:repo"}, - {"PUT", "/user/starred/:owner/:repo"}, - {"DELETE", "/user/starred/:owner/:repo"}, - {"GET", "/repos/:owner/:repo/subscribers"}, - {"GET", "/users/:user/subscriptions"}, - {"GET", "/user/subscriptions"}, - {"GET", "/repos/:owner/:repo/subscription"}, - {"PUT", "/repos/:owner/:repo/subscription"}, - {"DELETE", "/repos/:owner/:repo/subscription"}, - {"GET", "/user/subscriptions/:owner/:repo"}, - {"PUT", "/user/subscriptions/:owner/:repo"}, - {"DELETE", "/user/subscriptions/:owner/:repo"}, - - // Gists - {"GET", "/users/:user/gists"}, - {"GET", "/gists"}, - //{"GET", "/gists/public"}, - //{"GET", "/gists/starred"}, - {"GET", "/gists/:id"}, - {"POST", "/gists"}, - //{"PATCH", "/gists/:id"}, - {"PUT", "/gists/:id/star"}, - {"DELETE", "/gists/:id/star"}, - {"GET", "/gists/:id/star"}, - {"POST", "/gists/:id/forks"}, - {"DELETE", "/gists/:id"}, - - // Git Data - {"GET", "/repos/:owner/:repo/git/blobs/:sha"}, - {"POST", "/repos/:owner/:repo/git/blobs"}, - {"GET", "/repos/:owner/:repo/git/commits/:sha"}, - {"POST", "/repos/:owner/:repo/git/commits"}, - //{"GET", "/repos/:owner/:repo/git/refs/*ref"}, - {"GET", "/repos/:owner/:repo/git/refs"}, - {"POST", "/repos/:owner/:repo/git/refs"}, - //{"PATCH", "/repos/:owner/:repo/git/refs/*ref"}, - //{"DELETE", "/repos/:owner/:repo/git/refs/*ref"}, - {"GET", "/repos/:owner/:repo/git/tags/:sha"}, - {"POST", "/repos/:owner/:repo/git/tags"}, - {"GET", "/repos/:owner/:repo/git/trees/:sha"}, - {"POST", "/repos/:owner/:repo/git/trees"}, - - // Issues - {"GET", "/issues"}, - {"GET", "/user/issues"}, - {"GET", "/orgs/:org/issues"}, - {"GET", "/repos/:owner/:repo/issues"}, - {"GET", "/repos/:owner/:repo/issues/:number"}, - {"POST", "/repos/:owner/:repo/issues"}, - //{"PATCH", "/repos/:owner/:repo/issues/:number"}, - {"GET", "/repos/:owner/:repo/assignees"}, - {"GET", "/repos/:owner/:repo/assignees/:assignee"}, - {"GET", "/repos/:owner/:repo/issues/:number/comments"}, - //{"GET", "/repos/:owner/:repo/issues/comments"}, - //{"GET", "/repos/:owner/:repo/issues/comments/:id"}, - {"POST", "/repos/:owner/:repo/issues/:number/comments"}, - //{"PATCH", "/repos/:owner/:repo/issues/comments/:id"}, - //{"DELETE", "/repos/:owner/:repo/issues/comments/:id"}, - {"GET", "/repos/:owner/:repo/issues/:number/events"}, - //{"GET", "/repos/:owner/:repo/issues/events"}, - //{"GET", "/repos/:owner/:repo/issues/events/:id"}, - {"GET", "/repos/:owner/:repo/labels"}, - {"GET", "/repos/:owner/:repo/labels/:name"}, - {"POST", "/repos/:owner/:repo/labels"}, - //{"PATCH", "/repos/:owner/:repo/labels/:name"}, - {"DELETE", "/repos/:owner/:repo/labels/:name"}, - {"GET", "/repos/:owner/:repo/issues/:number/labels"}, - {"POST", "/repos/:owner/:repo/issues/:number/labels"}, - {"DELETE", "/repos/:owner/:repo/issues/:number/labels/:name"}, - {"PUT", "/repos/:owner/:repo/issues/:number/labels"}, - {"DELETE", "/repos/:owner/:repo/issues/:number/labels"}, - {"GET", "/repos/:owner/:repo/milestones/:number/labels"}, - {"GET", "/repos/:owner/:repo/milestones"}, - {"GET", "/repos/:owner/:repo/milestones/:number"}, - {"POST", "/repos/:owner/:repo/milestones"}, - //{"PATCH", "/repos/:owner/:repo/milestones/:number"}, - {"DELETE", "/repos/:owner/:repo/milestones/:number"}, - - // Miscellaneous - {"GET", "/emojis"}, - {"GET", "/gitignore/templates"}, - {"GET", "/gitignore/templates/:name"}, - {"POST", "/markdown"}, - {"POST", "/markdown/raw"}, - {"GET", "/meta"}, - {"GET", "/rate_limit"}, - - // Organizations - {"GET", "/users/:user/orgs"}, - {"GET", "/user/orgs"}, - {"GET", "/orgs/:org"}, - //{"PATCH", "/orgs/:org"}, - {"GET", "/orgs/:org/members"}, - {"GET", "/orgs/:org/members/:user"}, - {"DELETE", "/orgs/:org/members/:user"}, - {"GET", "/orgs/:org/public_members"}, - {"GET", "/orgs/:org/public_members/:user"}, - {"PUT", "/orgs/:org/public_members/:user"}, - {"DELETE", "/orgs/:org/public_members/:user"}, - {"GET", "/orgs/:org/teams"}, - {"GET", "/teams/:id"}, - {"POST", "/orgs/:org/teams"}, - //{"PATCH", "/teams/:id"}, - {"DELETE", "/teams/:id"}, - {"GET", "/teams/:id/members"}, - {"GET", "/teams/:id/members/:user"}, - {"PUT", "/teams/:id/members/:user"}, - {"DELETE", "/teams/:id/members/:user"}, - {"GET", "/teams/:id/repos"}, - {"GET", "/teams/:id/repos/:owner/:repo"}, - {"PUT", "/teams/:id/repos/:owner/:repo"}, - {"DELETE", "/teams/:id/repos/:owner/:repo"}, - {"GET", "/user/teams"}, + {"GET", "/events", "/events", nil}, + {"GET", "/repos/:owner/:repo/events", "/repos/dinever/golf/events", map[string]string{"owner": "dinever", "repo": "golf"}}, + {"GET", "/networks/:owner/:repo/events", "/networks/dinever/golf/events", map[string]string{"owner": "dinever", "repo": "golf"}}, + {"GET", "/orgs/:org/events", "/orgs/golf/events", map[string]string{"org": "golf"}}, + {"GET", "/users/:user/received_events", "/users/dinever/received_events", nil}, + {"GET", "/users/:user/received_events/public", "/users/dinever/received_events/public", nil}, +} - // Pull Requests - {"GET", "/repos/:owner/:repo/pulls"}, - {"GET", "/repos/:owner/:repo/pulls/:number"}, - {"POST", "/repos/:owner/:repo/pulls"}, - //{"PATCH", "/repos/:owner/:repo/pulls/:number"}, - {"GET", "/repos/:owner/:repo/pulls/:number/commits"}, - {"GET", "/repos/:owner/:repo/pulls/:number/files"}, - {"GET", "/repos/:owner/:repo/pulls/:number/merge"}, - {"PUT", "/repos/:owner/:repo/pulls/:number/merge"}, - {"GET", "/repos/:owner/:repo/pulls/:number/comments"}, - //{"GET", "/repos/:owner/:repo/pulls/comments"}, - //{"GET", "/repos/:owner/:repo/pulls/comments/:number"}, - {"PUT", "/repos/:owner/:repo/pulls/:number/comments"}, - //{"PATCH", "/repos/:owner/:repo/pulls/comments/:number"}, - //{"DELETE", "/repos/:owner/:repo/pulls/comments/:number"}, +func handler(ctx *Context) { +} - // Repositories - {"GET", "/user/repos"}, - {"GET", "/users/:user/repos"}, - {"GET", "/orgs/:org/repos"}, - {"GET", "/repositories"}, - {"POST", "/user/repos"}, - {"POST", "/orgs/:org/repos"}, - {"GET", "/repos/:owner/:repo"}, - //{"PATCH", "/repos/:owner/:repo"}, - {"GET", "/repos/:owner/:repo/contributors"}, - {"GET", "/repos/:owner/:repo/languages"}, - {"GET", "/repos/:owner/:repo/teams"}, - {"GET", "/repos/:owner/:repo/tags"}, - {"GET", "/repos/:owner/:repo/branches"}, - {"GET", "/repos/:owner/:repo/branches/:branch"}, - {"DELETE", "/repos/:owner/:repo"}, - {"GET", "/repos/:owner/:repo/collaborators"}, - {"GET", "/repos/:owner/:repo/collaborators/:user"}, - {"PUT", "/repos/:owner/:repo/collaborators/:user"}, - {"DELETE", "/repos/:owner/:repo/collaborators/:user"}, - {"GET", "/repos/:owner/:repo/comments"}, - {"GET", "/repos/:owner/:repo/commits/:sha/comments"}, - {"POST", "/repos/:owner/:repo/commits/:sha/comments"}, - {"GET", "/repos/:owner/:repo/comments/:id"}, - //{"PATCH", "/repos/:owner/:repo/comments/:id"}, - {"DELETE", "/repos/:owner/:repo/comments/:id"}, - {"GET", "/repos/:owner/:repo/commits"}, - {"GET", "/repos/:owner/:repo/commits/:sha"}, - {"GET", "/repos/:owner/:repo/readme"}, - //{"GET", "/repos/:owner/:repo/contents/*path"}, - //{"PUT", "/repos/:owner/:repo/contents/*path"}, - //{"DELETE", "/repos/:owner/:repo/contents/*path"}, - //{"GET", "/repos/:owner/:repo/:archive_format/:ref"}, - {"GET", "/repos/:owner/:repo/keys"}, - {"GET", "/repos/:owner/:repo/keys/:id"}, - {"POST", "/repos/:owner/:repo/keys"}, - //{"PATCH", "/repos/:owner/:repo/keys/:id"}, - {"DELETE", "/repos/:owner/:repo/keys/:id"}, - {"GET", "/repos/:owner/:repo/downloads"}, - {"GET", "/repos/:owner/:repo/downloads/:id"}, - {"DELETE", "/repos/:owner/:repo/downloads/:id"}, - {"GET", "/repos/:owner/:repo/forks"}, - {"POST", "/repos/:owner/:repo/forks"}, - {"GET", "/repos/:owner/:repo/hooks"}, - {"GET", "/repos/:owner/:repo/hooks/:id"}, - {"POST", "/repos/:owner/:repo/hooks"}, - //{"PATCH", "/repos/:owner/:repo/hooks/:id"}, - {"POST", "/repos/:owner/:repo/hooks/:id/tests"}, - {"DELETE", "/repos/:owner/:repo/hooks/:id"}, - {"POST", "/repos/:owner/:repo/merges"}, - {"GET", "/repos/:owner/:repo/releases"}, - {"GET", "/repos/:owner/:repo/releases/:id"}, - {"POST", "/repos/:owner/:repo/releases"}, - //{"PATCH", "/repos/:owner/:repo/releases/:id"}, - {"DELETE", "/repos/:owner/:repo/releases/:id"}, - {"GET", "/repos/:owner/:repo/releases/:id/assets"}, - {"GET", "/repos/:owner/:repo/stats/contributors"}, - {"GET", "/repos/:owner/:repo/stats/commit_activity"}, - {"GET", "/repos/:owner/:repo/stats/code_frequency"}, - {"GET", "/repos/:owner/:repo/stats/participation"}, - {"GET", "/repos/:owner/:repo/stats/punch_card"}, - {"GET", "/repos/:owner/:repo/statuses/:ref"}, - {"POST", "/repos/:owner/:repo/statuses/:ref"}, +func TestRouter(t *testing.T) { + router := newRouter() + for _, route := range githubAPI { + router.AddRoute(route.method, route.path, handler) + } - // Search - {"GET", "/search/repositories"}, - {"GET", "/search/code"}, - {"GET", "/search/issues"}, - {"GET", "/search/users"}, - {"GET", "/legacy/issues/search/:owner/:repository/:state/:keyword"}, - {"GET", "/legacy/repos/search/:keyword"}, - {"GET", "/legacy/user/search/:keyword"}, - {"GET", "/legacy/user/email/:email"}, + for _, route := range githubAPI { + _, param, err := router.FindRoute(route.method, route.testPath) + if err != nil { + t.Errorf("Can not find route: %v", route.testPath) + } - // Users - {"GET", "/users/:user"}, - {"GET", "/user"}, - //{"PATCH", "/user"}, - {"GET", "/users"}, - {"GET", "/user/emails"}, - {"POST", "/user/emails"}, - {"DELETE", "/user/emails"}, - {"GET", "/users/:user/followers"}, - {"GET", "/user/followers"}, - {"GET", "/users/:user/following"}, - {"GET", "/user/following"}, - {"GET", "/user/following/:user"}, - {"GET", "/users/:user/following/:target_user"}, - {"PUT", "/user/following/:user"}, - {"DELETE", "/user/following/:user"}, - {"GET", "/users/:user/keys"}, - {"GET", "/user/keys"}, - {"GET", "/user/keys/:id"}, - {"POST", "/user/keys"}, - //{"PATCH", "/user/keys/:id"}, - {"DELETE", "/user/keys/:id"}, + for key, expected := range route.params { + val, err := param.ByName(key) + if err != nil { + t.Errorf("Can not retrieve parameter from route %v: %v", route.testPath, key) + } else { + assertStringEqual(t, expected, val) + } + val, err = param.ByName(key) + if err != nil { + t.Errorf("Can not retrieve parameter from route %v: %v", route.testPath, key) + } else { + assertStringEqual(t, expected, val) + } + } + } } +func TestSplitURLPath(t *testing.T) { -func handler(c context.Context, w http.ResponseWriter, r *http.Request) { - fmt.Fprintf(w, "Home") -} - + var table = map[string][2][]string{ + "/users/:name": {{"/users/", ":"}, {"name"}}, + "/users/:name/put": {{"/users/", ":", "/put"}, {"name"}}, + "/users/:name/put/:section": {{"/users/", ":", "/put/", ":"}, {"name", "section"}}, + "/customers/:name/put/:section": {{"/customers/", ":", "/put/", ":"}, {"name", "section"}}, + "/customers/groups/:name/put/:section": {{"/customers/groups/", ":", "/put/", ":"}, {"name", "section"}}, + } -func TestRouter(t *testing.T) { - router := NewRouter() - for _, route := range githubAPI { - router.AddRoute(route.method, route.path, handler) + for path, result := range table { + parts, _ := splitURLpath(path) + assertSliceEqual(t, parts, result[0]) } } diff --git a/session.go b/session.go index a623c1a..fd964b3 100644 --- a/session.go +++ b/session.go @@ -1,4 +1,4 @@ -package Golf +package golf import ( "crypto/rand" diff --git a/session_test.go b/session_test.go index 01e24d2..210e41d 100644 --- a/session_test.go +++ b/session_test.go @@ -1,4 +1,4 @@ -package Golf +package golf import ( "testing" diff --git a/template.go b/template.go index 39b0da0..d3e1ca0 100644 --- a/template.go +++ b/template.go @@ -1,4 +1,4 @@ -package Golf +package golf import ( "fmt" diff --git a/template_test.go b/template_test.go index f3b94bc..e4df404 100644 --- a/template_test.go +++ b/template_test.go @@ -1,4 +1,4 @@ -package Golf +package golf import ( "bytes" diff --git a/tree.go b/tree.go index 8e8c881..dfbf6c5 100644 --- a/tree.go +++ b/tree.go @@ -1,23 +1,23 @@ package golf import ( + "fmt" "sort" "strings" ) type Node struct { - text string - names map[string]int - handler Handler + text string + names map[string]int + handler Handler parent *Node - wildcard *Node colon *Node - nodes nodes - start byte - max byte - indices []uint8 + children nodes + start byte + max byte + indices []uint8 } type nodes []*Node @@ -36,13 +36,6 @@ func (s nodes) Less(i, j int) bool { func (n *Node) matchNode(path string) (*Node, int8, int) { - if path == "*" { - if n.wildcard == nil { - n.wildcard = &Node{text: "*"} - } - return n.wildcard, 0, 0 - } - if path == ":" { if n.colon == nil { n.colon = &Node{text: ":"} @@ -50,7 +43,7 @@ func (n *Node) matchNode(path string) (*Node, int8, int) { return n.colon, 0, 0 } - for i, node := range n.nodes { + for i, node := range n.children { if node.text[0] == path[0] { maxLength := len(node.text) @@ -66,10 +59,10 @@ func (n *Node) matchNode(path string) (*Node, int8, int) { for j := 0; j < maxLength; j++ { if path[j] != node.text[j] { - ccNode := &Node{text: path[0:j], nodes: nodes{node, &Node{text: path[j:]}}} + ccNode := &Node{text: path[0:j], children: nodes{node, &Node{text: path[j:]}}} node.text = node.text[j:] - n.nodes[i] = ccNode - return ccNode.nodes[1], 0, i + n.children[i] = ccNode + return ccNode.children[1], 0, i } } @@ -93,7 +86,7 @@ func (n *Node) addRoute(parts []string, names map[string]int, handler Handler) { for loop == true { if currentNode == nil { currentNode = &Node{text: parts[0]} - n.nodes = append(n.nodes, currentNode) + n.children = append(n.children, currentNode) } else if result == 1 { // parts[0] = parts[0][len(currentNode.text):] @@ -104,8 +97,8 @@ func (n *Node) addRoute(parts []string, names map[string]int, handler Handler) { } else if result == -1 { tmpNode := &Node{text: parts[0]} currentNode.text = currentNode.text[len(tmpNode.text):] - tmpNode.nodes = nodes{currentNode} - n.nodes[i] = tmpNode + tmpNode.children = nodes{currentNode} + n.children[i] = tmpNode currentNode = tmpNode } break @@ -120,26 +113,23 @@ func (n *Node) addRoute(parts []string, names map[string]int, handler Handler) { currentNode.addRoute(parts[1:], names, handler) } -func (n *Node) findRoute(urlPath string) (*Node, int) { +func (n *Node) findRoute(urlPath string) (*Node, error) { urlByte := urlPath[0] pathLen := len(urlPath) if urlByte >= n.start && urlByte <= n.max { if i := n.indices[urlByte-n.start]; i != 0 { - matched := n.nodes[i-1] + matched := n.children[i-1] nodeLen := len(matched.text) if nodeLen < pathLen { if matched.text == urlPath[:nodeLen] { - if matched, wildcard := matched.findRoute(urlPath[nodeLen:]); matched != nil { - return matched, wildcard + if matched, _ := matched.findRoute(urlPath[nodeLen:]); matched != nil { + return matched, nil } } } else if matched.text == urlPath { - if matched.handler == nil && matched.wildcard != nil { - return matched.wildcard, 0 - } - return matched, 0 + return matched, nil } } } @@ -147,34 +137,30 @@ func (n *Node) findRoute(urlPath string) (*Node, int) { if n.colon != nil && pathLen != 0 { i := strings.IndexByte(urlPath, '/') if i > 0 { - if cNode, wildcard := n.colon.findRoute(urlPath[i:]); cNode != nil { - return cNode, wildcard + if cNode, err := n.colon.findRoute(urlPath[i:]); cNode != nil { + return cNode, err } } else if n.colon.handler != nil { - return n.colon, 0 + return n.colon, nil } } - if n.wildcard != nil { - return n.wildcard, pathLen - } - - return nil, 0 + return nil, fmt.Errorf("Can not find route") } func (n *Node) optimizeRoutes() { - if len(n.nodes) > 0 { - sort.Sort(n.nodes) + if len(n.children) > 0 { + sort.Sort(n.children) for i := 0; i < len(n.indices); i++ { n.indices[i] = 0 } - n.start = n.nodes[0].text[0] - n.max = n.nodes[len(n.nodes)-1].text[0] + n.start = n.children[0].text[0] + n.max = n.children[len(n.children)-1].text[0] - for i := 0; i < len(n.nodes); i++ { - cNode := n.nodes[i] + for i := 0; i < len(n.children); i++ { + cNode := n.children[i] cNode.parent = n cByte := int(cNode.text[0] - n.start) @@ -190,49 +176,38 @@ func (n *Node) optimizeRoutes() { n.colon.parent = n n.colon.optimizeRoutes() } - - if n.wildcard != nil { - n.wildcard.parent = n - n.wildcard.optimizeRoutes() - } } -func (_node *Node) finalize() { - if len(_node.nodes) > 0 { - for i := 0; i < len(_node.nodes); i++ { - _node.nodes[i].finalize() +func (n *Node) finalize() { + if len(n.children) > 0 { + for i := 0; i < len(n.children); i++ { + n.children[i].finalize() } } - if _node.colon != nil { - _node.colon.finalize() - } - if _node.wildcard != nil { - _node.wildcard.finalize() + if n.colon != nil { + n.colon.finalize() } - *_node = Node{} + *n = Node{} } -func (_node *Node) string(col int) string { - var str = "\n" + strings.Repeat(" ", col) + _node.text + " -> " - col += len(_node.text) + 4 - for i := 0; i < len(_node.indices); i++ { - if j := _node.indices[i]; j != 0 { - str += _node.nodes[j-1].string(col) +func (n *Node) string(col int) string { + var str = "\n" + strings.Repeat(" ", col) + n.text + " -> " + col += len(n.text) + 4 + for i := 0; i < len(n.indices); i++ { + if j := n.indices[i]; j != 0 { + str += n.children[j-1].string(col) } } - if _node.colon != nil { - str += _node.colon.string(col) - } - if _node.wildcard != nil { - str += _node.wildcard.string(col) + if n.colon != nil { + str += n.colon.string(col) } return str } -func (_node *Node) String() string { - if _node.text == "" { - return _node.string(0) +func (n *Node) String() string { + if n.text == "" { + return n.string(0) } - col := len(_node.text) + 4 - return _node.text + " -> " + _node.string(col) + col := len(n.text) + 4 + return n.text + " -> " + n.string(col) } diff --git a/view.go b/view.go index 60b14e4..8a137bc 100644 --- a/view.go +++ b/view.go @@ -1,4 +1,4 @@ -package Golf +package golf import ( "bytes" diff --git a/view_test.go b/view_test.go index 8cba5ab..904f931 100644 --- a/view_test.go +++ b/view_test.go @@ -1,4 +1,4 @@ -package Golf +package golf import ( "testing" diff --git a/xsrf.go b/xsrf.go index dc5adc3..03d566d 100644 --- a/xsrf.go +++ b/xsrf.go @@ -1,4 +1,4 @@ -package Golf +package golf import ( "encoding/hex" diff --git a/xsrf_test.go b/xsrf_test.go index 887baed..6cc078f 100644 --- a/xsrf_test.go +++ b/xsrf_test.go @@ -1,4 +1,4 @@ -package Golf +package golf import ( "encoding/hex" From db3fa0f2ce728e43373d8f0c5492e9411c818c8e Mon Sep 17 00:00:00 2001 From: dinever Date: Wed, 20 Apr 2016 23:16:13 -0400 Subject: [PATCH 03/15] [fix] Remove strings.LastIndexByte usage --- router.go | 16 ++++++++++++---- tree.go | 10 +++++----- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/router.go b/router.go index 0189e0f..d85f659 100644 --- a/router.go +++ b/router.go @@ -2,7 +2,6 @@ package golf import ( "fmt" - "strings" ) // Handler is the type of the handler function that Golf accepts. @@ -107,8 +106,8 @@ func (router *router) String() string { //Parameter holds the parameters matched in the route type Parameter struct { - *Node // matched node - path string // url path given + *Node // matched node + path string // url path given cached map[string]string } @@ -125,6 +124,15 @@ func (p *Parameter) ByName(name string) (string, error) { return "", fmt.Errorf("Parameter not found") } +func lastIndexByte(s string, c byte) int { + for i := len(s) - 1; i >= 0; i-- { + if s[i] == c { + return i + } + } + return -1 +} + //findParam walks up the matched node looking for parameters returns the last parameter func (p *Parameter) findParam(idx int) (string, error) { index := len(p.names) - 1 @@ -134,7 +142,7 @@ func (p *Parameter) findParam(idx int) (string, error) { for node != nil { if node.text[0] == ':' { - ctn := strings.LastIndexByte(urlPath, '/') + ctn := lastIndexByte(urlPath, '/') if ctn == -1 { return "", fmt.Errorf("Parameter not found") } diff --git a/tree.go b/tree.go index dfbf6c5..f026b08 100644 --- a/tree.go +++ b/tree.go @@ -7,12 +7,12 @@ import ( ) type Node struct { - text string - names map[string]int - handler Handler + text string + names map[string]int + handler Handler - parent *Node - colon *Node + parent *Node + colon *Node children nodes start byte From a6bde2481ffd1be9e9a2d7aa70ad14e130879f8e Mon Sep 17 00:00:00 2001 From: dinever Date: Fri, 22 Apr 2016 01:41:59 -0400 Subject: [PATCH 04/15] [feat] Add a pool of context to reduce allocation --- app.go | 45 ++++++++++++++++++++++++++++----------------- middleware.go | 12 ++++++------ router.go | 8 ++++---- tree.go | 12 ++++++------ 4 files changed, 44 insertions(+), 33 deletions(-) diff --git a/app.go b/app.go index b294a9a..a6f541c 100644 --- a/app.go +++ b/app.go @@ -5,36 +5,39 @@ import ( "os" "path" "strings" + "sync" ) // Application is an abstraction of a Golf application, can be used for // configuration, etc. type Application struct { - router *router + router *router // A map of string slices as value to indicate the static files. - staticRouter map[string][]string + staticRouter map[string][]string // The View model of the application. View handles the templating and page // rendering. - View *View + View *View // Config provides configuration management. - Config *Config + Config *Config - SessionManager SessionManager + SessionManager SessionManager // NotFoundHandler handles requests when no route is matched. - NotFoundHandler Handler + NotFoundHandler HandlerFunc // MiddlewareChain is the default middlewares that Golf uses. - MiddlewareChain *Chain + MiddlewareChain *Chain - errorHandler map[int]ErrorHandlerType + pool sync.Pool + + errorHandler map[int]ErrorHandlerFunc // The default error handler, if the corresponding error code is not specified // in the `errorHandler` map, this handler will be called. - DefaultErrorHandler ErrorHandlerType + DefaultErrorHandler ErrorHandlerFunc } // New is used for creating a new Golf Application instance. @@ -45,9 +48,12 @@ func New() *Application { app.View = NewView() app.Config = NewConfig(app) // debug, _ := app.Config.GetBool("debug", false) - app.errorHandler = make(map[int]ErrorHandlerType) - app.MiddlewareChain = NewChain(defaultMiddlewares...) + app.errorHandler = make(map[int]ErrorHandlerFunc) + app.MiddlewareChain = NewChain() app.DefaultErrorHandler = defaultErrorHandler + app.pool.New = func() interface{} { + return new(Context) + } return app } @@ -84,8 +90,13 @@ func staticHandler(ctx *Context, filePath string) { // Basic entrance of an `http.ResponseWriter` and an `http.Request`. func (app *Application) ServeHTTP(res http.ResponseWriter, req *http.Request) { - ctx := NewContext(req, res, app) + // ctx := NewContext(req, res, app) + ctx := app.pool.Get().(*Context) + ctx.Request = req + ctx.Response = res + ctx.App = app app.MiddlewareChain.Final(app.handler)(ctx) + app.pool.Put(ctx) } // Run the Golf Application. @@ -111,27 +122,27 @@ func (app *Application) Static(url string, path string) { } // Get method is used for registering a Get method route -func (app *Application) Get(pattern string, handler Handler) { +func (app *Application) Get(pattern string, handler HandlerFunc) { app.router.AddRoute("GET", pattern, handler) } // Post method is used for registering a Post method route -func (app *Application) Post(pattern string, handler Handler) { +func (app *Application) Post(pattern string, handler HandlerFunc) { app.router.AddRoute("POST", pattern, handler) } // Put method is used for registering a Put method route -func (app *Application) Put(pattern string, handler Handler) { +func (app *Application) Put(pattern string, handler HandlerFunc) { app.router.AddRoute("PUT", pattern, handler) } // Delete method is used for registering a Delete method route -func (app *Application) Delete(pattern string, handler Handler) { +func (app *Application) Delete(pattern string, handler HandlerFunc) { app.router.AddRoute("DELETE", pattern, handler) } // Error method is used for registering an handler for a specified HTTP error code. -func (app *Application) Error(statusCode int, handler ErrorHandlerType) { +func (app *Application) Error(statusCode int, handler ErrorHandlerFunc) { app.errorHandler[statusCode] = handler } diff --git a/middleware.go b/middleware.go index 99e8c13..762538c 100644 --- a/middleware.go +++ b/middleware.go @@ -6,7 +6,7 @@ import ( "time" ) -type middlewareHandler func(next Handler) Handler +type middlewareHandler func(next HandlerFunc) HandlerFunc var defaultMiddlewares = []middlewareHandler{LoggingMiddleware, RecoverMiddleware, XSRFProtectionMiddleware, SessionMiddleware} @@ -24,7 +24,7 @@ func NewChain(handlerArray ...middlewareHandler) *Chain { // Final indicates a final Handler, chain the multiple middlewares together with the // handler, and return them together as a handler. -func (c Chain) Final(fn Handler) Handler { +func (c Chain) Final(fn HandlerFunc) HandlerFunc { final := fn for i := len(c.middlewareHandlers) - 1; i >= 0; i-- { final = c.middlewareHandlers[i](final) @@ -38,7 +38,7 @@ func (c *Chain) Append(fn middlewareHandler) { } // LoggingMiddleware is the built-in middleware for logging. -func LoggingMiddleware(next Handler) Handler { +func LoggingMiddleware(next HandlerFunc) HandlerFunc { fn := func(ctx *Context) { t1 := time.Now() next(ctx) @@ -49,7 +49,7 @@ func LoggingMiddleware(next Handler) Handler { } // XSRFProtectionMiddleware is the built-in middleware for XSRF protection. -func XSRFProtectionMiddleware(next Handler) Handler { +func XSRFProtectionMiddleware(next HandlerFunc) HandlerFunc { fn := func(ctx *Context) { xsrfEnabled, _ := ctx.App.Config.GetBool("xsrf_cookies", false) if xsrfEnabled && (ctx.Request.Method == "POST" || ctx.Request.Method == "PUT" || ctx.Request.Method == "DELETE") { @@ -64,7 +64,7 @@ func XSRFProtectionMiddleware(next Handler) Handler { } // RecoverMiddleware is the built-in middleware for recovering from errors. -func RecoverMiddleware(next Handler) Handler { +func RecoverMiddleware(next HandlerFunc) HandlerFunc { fn := func(ctx *Context) { defer func() { if err := recover(); err != nil { @@ -86,7 +86,7 @@ func RecoverMiddleware(next Handler) Handler { return fn } -func SessionMiddleware(next Handler) Handler { +func SessionMiddleware(next HandlerFunc) HandlerFunc { fn := func(ctx *Context) { if ctx.App.SessionManager != nil { ctx.retrieveSession() diff --git a/router.go b/router.go index d85f659..adc683b 100644 --- a/router.go +++ b/router.go @@ -5,10 +5,10 @@ import ( ) // Handler is the type of the handler function that Golf accepts. -type Handler func(ctx *Context) +type HandlerFunc func(ctx *Context) // ErrorHandlerType is the type of the function that handles error in Golf. -type ErrorHandlerType func(ctx *Context, data ...map[string]interface{}) +type ErrorHandlerFunc func(ctx *Context, data ...map[string]interface{}) type router struct { trees map[string]*Node @@ -70,7 +70,7 @@ func (router *router) Finalize() { } } -func (router *router) FindRoute(method string, path string) (Handler, Parameter, error) { +func (router *router) FindRoute(method string, path string) (HandlerFunc, Parameter, error) { node := router.trees[method] if node == nil { return nil, Parameter{}, fmt.Errorf("Can not find route") @@ -82,7 +82,7 @@ func (router *router) FindRoute(method string, path string) (Handler, Parameter, return matchedNode.handler, Parameter{Node: matchedNode, path: path}, err } -func (router *router) AddRoute(method string, path string, handler Handler) { +func (router *router) AddRoute(method string, path string, handler HandlerFunc) { var ( rootNode *Node ok bool diff --git a/tree.go b/tree.go index f026b08..6656282 100644 --- a/tree.go +++ b/tree.go @@ -7,12 +7,12 @@ import ( ) type Node struct { - text string - names map[string]int - handler Handler + text string + names map[string]int + handler HandlerFunc - parent *Node - colon *Node + parent *Node + colon *Node children nodes start byte @@ -73,7 +73,7 @@ func (n *Node) matchNode(path string) (*Node, int8, int) { return nil, 0, 0 } -func (n *Node) addRoute(parts []string, names map[string]int, handler Handler) { +func (n *Node) addRoute(parts []string, names map[string]int, handler HandlerFunc) { var ( tmpNode *Node From 440d938d13f395a72ed13c9729a27641334db47b Mon Sep 17 00:00:00 2001 From: dinever Date: Fri, 22 Apr 2016 02:25:47 -0400 Subject: [PATCH 05/15] [refactor] Improved coding style --- app.go | 24 +++++++++++++---------- middleware.go | 6 +++--- router.go | 18 ++++++++--------- session.go | 1 + tree.go | 54 +++++++++++++++++++++++++-------------------------- 5 files changed, 54 insertions(+), 49 deletions(-) diff --git a/app.go b/app.go index a6f541c..3bcdddc 100644 --- a/app.go +++ b/app.go @@ -11,33 +11,35 @@ import ( // Application is an abstraction of a Golf application, can be used for // configuration, etc. type Application struct { - router *router + router *router // A map of string slices as value to indicate the static files. - staticRouter map[string][]string + staticRouter map[string][]string // The View model of the application. View handles the templating and page // rendering. - View *View + View *View // Config provides configuration management. - Config *Config + Config *Config - SessionManager SessionManager + SessionManager SessionManager // NotFoundHandler handles requests when no route is matched. - NotFoundHandler HandlerFunc + NotFoundHandler HandlerFunc // MiddlewareChain is the default middlewares that Golf uses. - MiddlewareChain *Chain + MiddlewareChain *Chain pool sync.Pool - errorHandler map[int]ErrorHandlerFunc + errorHandler map[int]ErrorHandlerFunc // The default error handler, if the corresponding error code is not specified // in the `errorHandler` map, this handler will be called. DefaultErrorHandler ErrorHandlerFunc + + handlerChain HandlerFunc } // New is used for creating a new Golf Application instance. @@ -90,12 +92,14 @@ func staticHandler(ctx *Context, filePath string) { // Basic entrance of an `http.ResponseWriter` and an `http.Request`. func (app *Application) ServeHTTP(res http.ResponseWriter, req *http.Request) { - // ctx := NewContext(req, res, app) + if app.handlerChain == nil { + app.handlerChain = app.MiddlewareChain.Final(app.handler) + } ctx := app.pool.Get().(*Context) ctx.Request = req ctx.Response = res ctx.App = app - app.MiddlewareChain.Final(app.handler)(ctx) + app.handlerChain(ctx) app.pool.Put(ctx) } diff --git a/middleware.go b/middleware.go index 762538c..abd9ac1 100644 --- a/middleware.go +++ b/middleware.go @@ -25,11 +25,10 @@ func NewChain(handlerArray ...middlewareHandler) *Chain { // Final indicates a final Handler, chain the multiple middlewares together with the // handler, and return them together as a handler. func (c Chain) Final(fn HandlerFunc) HandlerFunc { - final := fn for i := len(c.middlewareHandlers) - 1; i >= 0; i-- { - final = c.middlewareHandlers[i](final) + fn = c.middlewareHandlers[i](fn) } - return final + return fn } // Append a middleware to the middleware chain @@ -86,6 +85,7 @@ func RecoverMiddleware(next HandlerFunc) HandlerFunc { return fn } +// SessionMiddleware handles session of the request func SessionMiddleware(next HandlerFunc) HandlerFunc { fn := func(ctx *Context) { if ctx.App.SessionManager != nil { diff --git a/router.go b/router.go index adc683b..34411f6 100644 --- a/router.go +++ b/router.go @@ -4,18 +4,18 @@ import ( "fmt" ) -// Handler is the type of the handler function that Golf accepts. +// HandlerFunc is the type of the handler function that Golf accepts. type HandlerFunc func(ctx *Context) -// ErrorHandlerType is the type of the function that handles error in Golf. +// ErrorHandlerFunc is the type of the function that handles error in Golf. type ErrorHandlerFunc func(ctx *Context, data ...map[string]interface{}) type router struct { - trees map[string]*Node + trees map[string]*node } func newRouter() *router { - return &router{trees: make(map[string]*Node)} + return &router{trees: make(map[string]*node)} } func splitURLpath(path string) (parts []string, names map[string]int) { @@ -79,17 +79,17 @@ func (router *router) FindRoute(method string, path string) (HandlerFunc, Parame if err != nil { return nil, Parameter{}, err } - return matchedNode.handler, Parameter{Node: matchedNode, path: path}, err + return matchedNode.handler, Parameter{node: matchedNode, path: path}, err } func (router *router) AddRoute(method string, path string, handler HandlerFunc) { var ( - rootNode *Node + rootNode *node ok bool ) parts, names := splitURLpath(path) if rootNode, ok = router.trees[method]; !ok { - rootNode = &Node{} + rootNode = &node{} router.trees[method] = rootNode } rootNode.addRoute(parts, names, handler) @@ -106,7 +106,7 @@ func (router *router) String() string { //Parameter holds the parameters matched in the route type Parameter struct { - *Node // matched node + *node // matched node path string // url path given cached map[string]string } @@ -138,7 +138,7 @@ func (p *Parameter) findParam(idx int) (string, error) { index := len(p.names) - 1 urlPath := p.path pathLen := len(p.path) - node := p.Node + node := p.node for node != nil { if node.text[0] == ':' { diff --git a/session.go b/session.go index fd964b3..ff8b1cd 100644 --- a/session.go +++ b/session.go @@ -79,6 +79,7 @@ func (mgr *MemorySessionManager) GarbageCollection() { time.AfterFunc(time.Duration(gcTimeInterval)*time.Second, mgr.GarbageCollection) } +// Count returns the number of the current session stored in the session manager. func (mgr *MemorySessionManager) Count() int { return len(mgr.sessions) } diff --git a/tree.go b/tree.go index 6656282..ae6964e 100644 --- a/tree.go +++ b/tree.go @@ -6,13 +6,13 @@ import ( "strings" ) -type Node struct { - text string - names map[string]int - handler HandlerFunc +type node struct { + text string + names map[string]int + handler HandlerFunc - parent *Node - colon *Node + parent *node + colon *node children nodes start byte @@ -20,7 +20,7 @@ type Node struct { indices []uint8 } -type nodes []*Node +type nodes []*node func (s nodes) Len() int { return len(s) @@ -34,19 +34,19 @@ func (s nodes) Less(i, j int) bool { return s[i].text[0] < s[j].text[0] } -func (n *Node) matchNode(path string) (*Node, int8, int) { +func (n *node) matchNode(path string) (*node, int8, int) { if path == ":" { if n.colon == nil { - n.colon = &Node{text: ":"} + n.colon = &node{text: ":"} } return n.colon, 0, 0 } - for i, node := range n.children { - if node.text[0] == path[0] { + for i, child := range n.children { + if child.text[0] == path[0] { - maxLength := len(node.text) + maxLength := len(child.text) pathLength := len(path) var pathCompare int8 @@ -58,26 +58,26 @@ func (n *Node) matchNode(path string) (*Node, int8, int) { } for j := 0; j < maxLength; j++ { - if path[j] != node.text[j] { - ccNode := &Node{text: path[0:j], children: nodes{node, &Node{text: path[j:]}}} - node.text = node.text[j:] + if path[j] != child.text[j] { + ccNode := &node{text: path[0:j], children: nodes{child, &node{text: path[j:]}}} + child.text = child.text[j:] n.children[i] = ccNode return ccNode.children[1], 0, i } } - return node, pathCompare, i + return child, pathCompare, i } } return nil, 0, 0 } -func (n *Node) addRoute(parts []string, names map[string]int, handler HandlerFunc) { +func (n *node) addRoute(parts []string, names map[string]int, handler HandlerFunc) { var ( - tmpNode *Node - currentNode *Node + tmpNode *node + currentNode *node loop = true ) @@ -85,7 +85,7 @@ func (n *Node) addRoute(parts []string, names map[string]int, handler HandlerFun for loop == true { if currentNode == nil { - currentNode = &Node{text: parts[0]} + currentNode = &node{text: parts[0]} n.children = append(n.children, currentNode) } else if result == 1 { // @@ -95,7 +95,7 @@ func (n *Node) addRoute(parts []string, names map[string]int, handler HandlerFun currentNode = tmpNode continue } else if result == -1 { - tmpNode := &Node{text: parts[0]} + tmpNode := &node{text: parts[0]} currentNode.text = currentNode.text[len(tmpNode.text):] tmpNode.children = nodes{currentNode} n.children[i] = tmpNode @@ -113,7 +113,7 @@ func (n *Node) addRoute(parts []string, names map[string]int, handler HandlerFun currentNode.addRoute(parts[1:], names, handler) } -func (n *Node) findRoute(urlPath string) (*Node, error) { +func (n *node) findRoute(urlPath string) (*node, error) { urlByte := urlPath[0] pathLen := len(urlPath) @@ -148,7 +148,7 @@ func (n *Node) findRoute(urlPath string) (*Node, error) { return nil, fmt.Errorf("Can not find route") } -func (n *Node) optimizeRoutes() { +func (n *node) optimizeRoutes() { if len(n.children) > 0 { sort.Sort(n.children) @@ -178,7 +178,7 @@ func (n *Node) optimizeRoutes() { } } -func (n *Node) finalize() { +func (n *node) finalize() { if len(n.children) > 0 { for i := 0; i < len(n.children); i++ { n.children[i].finalize() @@ -187,10 +187,10 @@ func (n *Node) finalize() { if n.colon != nil { n.colon.finalize() } - *n = Node{} + *n = node{} } -func (n *Node) string(col int) string { +func (n *node) string(col int) string { var str = "\n" + strings.Repeat(" ", col) + n.text + " -> " col += len(n.text) + 4 for i := 0; i < len(n.indices); i++ { @@ -204,7 +204,7 @@ func (n *Node) string(col int) string { return str } -func (n *Node) String() string { +func (n *node) String() string { if n.text == "" { return n.string(0) } From 02a2e9b4e84ce9bb8afce975e7490cce4eae23f8 Mon Sep 17 00:00:00 2001 From: dinever Date: Fri, 22 Apr 2016 02:46:44 -0400 Subject: [PATCH 06/15] [fix] Reset context before using it --- app.go | 1 + context.go | 5 +++++ xsrf_test.go | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app.go b/app.go index 3bcdddc..1f5d969 100644 --- a/app.go +++ b/app.go @@ -96,6 +96,7 @@ func (app *Application) ServeHTTP(res http.ResponseWriter, req *http.Request) { app.handlerChain = app.MiddlewareChain.Final(app.handler) } ctx := app.pool.Get().(*Context) + ctx.reset() ctx.Request = req ctx.Response = res ctx.App = app diff --git a/context.go b/context.go index 5192397..1ab8602 100644 --- a/context.go +++ b/context.go @@ -60,6 +60,11 @@ func NewContext(req *http.Request, res http.ResponseWriter, app *Application) *C return ctx } +func (ctx *Context) reset() { + ctx.StatusCode = 200 + ctx.IsSent = false +} + func (ctx *Context) generateSession() Session { s, err := ctx.App.SessionManager.NewSession() if err != nil { diff --git a/xsrf_test.go b/xsrf_test.go index 6cc078f..011c1a5 100644 --- a/xsrf_test.go +++ b/xsrf_test.go @@ -20,6 +20,6 @@ func TestDecodeXSRF(t *testing.T) { maskedTokenBytes, _ := hex.DecodeString(maskedToken) mask, token, _ := decodeXSRFToken(maskedToken) if !compareToken(maskedTokenBytes, append(mask, websocketMask(mask, token)...)) { - t.Errorf("Could not genearte correct XSRF token. %v != %v") + t.Error("Could not genearte correct XSRF token. %v != %v") } } From 4ea46a98a3b1cfffc0bbf61a9e716537932b8e6c Mon Sep 17 00:00:00 2001 From: dinever Date: Fri, 22 Apr 2016 03:06:11 -0400 Subject: [PATCH 07/15] [feat] Update vet and cover for travis --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index a90dc99..ee0952f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,5 +11,6 @@ before_install: - go get golang.org/x/tools/cmd/cover script: + - go vet ./... - go test -v -covermode=count -coverprofile=coverage.out From 9c0197f69a64c44f0e9ca127f87306e1a417e0dc Mon Sep 17 00:00:00 2001 From: dinever Date: Fri, 22 Apr 2016 03:17:37 -0400 Subject: [PATCH 08/15] [feat] Add Patch, Head, Options --- app.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/app.go b/app.go index 1f5d969..8ab3740 100644 --- a/app.go +++ b/app.go @@ -146,6 +146,21 @@ func (app *Application) Delete(pattern string, handler HandlerFunc) { app.router.AddRoute("DELETE", pattern, handler) } +// Patch method is used for registering a Patch method route +func (app *Application) Patch(pattern string, handler HandlerFunc) { + app.router.AddRoute("PATCH", pattern, handler) +} + +// Options method is used for registering a Options method route +func (app *Application) Options(pattern string, handler HandlerFunc) { + app.router.AddRoute("OPTIONS", pattern, handler) +} + +// Head method is used for registering a Head method route +func (app *Application) Head(pattern string, handler HandlerFunc) { + app.router.AddRoute("HEAD", pattern, handler) +} + // Error method is used for registering an handler for a specified HTTP error code. func (app *Application) Error(statusCode int, handler ErrorHandlerFunc) { app.errorHandler[statusCode] = handler From 6ee196faddc010e4248b771d013a3303655b1840 Mon Sep 17 00:00:00 2001 From: dinever Date: Fri, 22 Apr 2016 03:18:40 -0400 Subject: [PATCH 09/15] [fix] Fix error formatting --- xsrf_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/xsrf_test.go b/xsrf_test.go index 011c1a5..862e758 100644 --- a/xsrf_test.go +++ b/xsrf_test.go @@ -19,7 +19,8 @@ func TestDecodeXSRF(t *testing.T) { maskedToken := newXSRFToken() maskedTokenBytes, _ := hex.DecodeString(maskedToken) mask, token, _ := decodeXSRFToken(maskedToken) - if !compareToken(maskedTokenBytes, append(mask, websocketMask(mask, token)...)) { - t.Error("Could not genearte correct XSRF token. %v != %v") + result := append(mask, websocketMask(mask, token)...) + if !compareToken(maskedTokenBytes, result) { + t.Errorf("Could not genearte correct XSRF token. %v != %v", maskedTokenBytes, result) } } From 1cba56a0f005597bec82fb296c78e0d7503a6d49 Mon Sep 17 00:00:00 2001 From: dinever Date: Fri, 22 Apr 2016 08:33:31 -0400 Subject: [PATCH 10/15] [test] Improve test coverage --- router.go | 16 +--------------- router_test.go | 11 +++++++++++ tree.go | 34 ---------------------------------- 3 files changed, 12 insertions(+), 49 deletions(-) diff --git a/router.go b/router.go index 34411f6..f35f744 100644 --- a/router.go +++ b/router.go @@ -44,7 +44,7 @@ func splitURLpath(path string) (parts []string, names map[string]int) { } else { if path[i] == ':' || path[i] == '*' { if path[i-1] != '/' { - panic(fmt.Errorf("Inválid parameter : or * comes anwais after / - %q", path)) + panic(fmt.Errorf("Invalid parameter : or * should always be after / - %q", path)) } nameidx = i + 1 if partidx != i { @@ -64,12 +64,6 @@ func splitURLpath(path string) (parts []string, names map[string]int) { return } -func (router *router) Finalize() { - for _, _node := range router.trees { - _node.finalize() - } -} - func (router *router) FindRoute(method string, path string) (HandlerFunc, Parameter, error) { node := router.trees[method] if node == nil { @@ -96,14 +90,6 @@ func (router *router) AddRoute(method string, path string, handler HandlerFunc) rootNode.optimizeRoutes() } -func (router *router) String() string { - var lines string - for method, _node := range router.trees { - lines += method + " " + _node.String() - } - return lines -} - //Parameter holds the parameters matched in the route type Parameter struct { *node // matched node diff --git a/router_test.go b/router_test.go index 228b16e..03e18e9 100644 --- a/router_test.go +++ b/router_test.go @@ -94,3 +94,14 @@ func TestSplitURLPath(t *testing.T) { assertSliceEqual(t, parts, result[0]) } } + +func TestIncorrectPath(t *testing.T) { + path := "/users/foo:name/" + defer func() { + if err := recover(); err != nil { + } + }() + router := newRouter() + router.AddRoute("GET", path, handler) + t.Errorf("Incorrect path should raise an error.") +} diff --git a/tree.go b/tree.go index ae6964e..8cead84 100644 --- a/tree.go +++ b/tree.go @@ -177,37 +177,3 @@ func (n *node) optimizeRoutes() { n.colon.optimizeRoutes() } } - -func (n *node) finalize() { - if len(n.children) > 0 { - for i := 0; i < len(n.children); i++ { - n.children[i].finalize() - } - } - if n.colon != nil { - n.colon.finalize() - } - *n = node{} -} - -func (n *node) string(col int) string { - var str = "\n" + strings.Repeat(" ", col) + n.text + " -> " - col += len(n.text) + 4 - for i := 0; i < len(n.indices); i++ { - if j := n.indices[i]; j != 0 { - str += n.children[j-1].string(col) - } - } - if n.colon != nil { - str += n.colon.string(col) - } - return str -} - -func (n *node) String() string { - if n.text == "" { - return n.string(0) - } - col := len(n.text) + 4 - return n.text + " -> " + n.string(col) -} From 6343a5326de8a240d7bf962a8e74c1d8ba3d3bd1 Mon Sep 17 00:00:00 2001 From: dinever Date: Fri, 22 Apr 2016 08:40:05 -0400 Subject: [PATCH 11/15] [test] Improve test coverage --- router_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/router_test.go b/router_test.go index 03e18e9..4449741 100644 --- a/router_test.go +++ b/router_test.go @@ -31,6 +31,7 @@ type route struct { var githubAPI = []route{ // OAuth Authorizations {"GET", "/authorizations", "/authorizations", map[string]string{}}, + {"GET", "/auth", "/auth", map[string]string{}}, {"GET", "/authorizations/:id", "/authorizations/12345", map[string]string{"id": "12345"}}, {"POST", "/authorizations", "/authorizations", map[string]string{}}, {"DELETE", "/authorizations/:id", "/authorizations/12345", map[string]string{"id": "12345"}}, From 655d6bb3504f8f1640f32017699c27e19d18212b Mon Sep 17 00:00:00 2001 From: dinever Date: Fri, 22 Apr 2016 08:45:37 -0400 Subject: [PATCH 12/15] [test] Improve test coverage when path is not found --- router_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/router_test.go b/router_test.go index 4449741..aa5a221 100644 --- a/router_test.go +++ b/router_test.go @@ -106,3 +106,27 @@ func TestIncorrectPath(t *testing.T) { router.AddRoute("GET", path, handler) t.Errorf("Incorrect path should raise an error.") } + +func TestPathNotFound(t *testing.T) { + path := map[string]string { + "/users/name/": "/users/name/dinever/", + } + defer func() { + if err := recover(); err != nil { + } + }() + router := newRouter() + for route, wrongPath := range path { + router.AddRoute("GET", route, handler) + h, p, err := router.FindRoute("GET", wrongPath) + if h != nil { + t.Errorf("Should return nil handler when path not found.") + } + if p.Len() != 0 { + t.Errorf("Should return nil parameter when path not found.") + } + if err == nil { + t.Errorf("Should rasie an error when path not found.") + } + } +} From 9724bbbd43a893e5c93b37efb075f4d5c23d1508 Mon Sep 17 00:00:00 2001 From: dinever Date: Fri, 22 Apr 2016 08:49:06 -0400 Subject: [PATCH 13/15] [test] Improve test coverage when method is not found --- router_test.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/router_test.go b/router_test.go index aa5a221..4c6ccaa 100644 --- a/router_test.go +++ b/router_test.go @@ -108,17 +108,20 @@ func TestIncorrectPath(t *testing.T) { } func TestPathNotFound(t *testing.T) { - path := map[string]string { - "/users/name/": "/users/name/dinever/", + path := []struct { + method, path, incomingMethod, incomingPath string + } { + {"GET", "/users/name/", "GET", "/users/name/dinever/"}, + {"GET", "/dinever/repo/", "POST", "/dinever/repo/"}, } defer func() { if err := recover(); err != nil { } }() router := newRouter() - for route, wrongPath := range path { - router.AddRoute("GET", route, handler) - h, p, err := router.FindRoute("GET", wrongPath) + for _, path := range path { + router.AddRoute(path.method, path.path, handler) + h, p, err := router.FindRoute(path.incomingMethod, path.incomingPath) if h != nil { t.Errorf("Should return nil handler when path not found.") } From 06d7a90f5d8629fea5f936f3a09169ab8869a7c2 Mon Sep 17 00:00:00 2001 From: dinever Date: Fri, 22 Apr 2016 09:14:41 -0400 Subject: [PATCH 14/15] [test] Improve test coverage for Config --- config.go | 28 +++++++-------- config_test.go | 94 +++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 104 insertions(+), 18 deletions(-) diff --git a/config.go b/config.go index 305506e..0359353 100644 --- a/config.go +++ b/config.go @@ -38,61 +38,61 @@ type Config struct { } // NewConfig creates a new configuration instance. -func NewConfig(app *Application) *Config { +func NewConfig() *Config { mapping := make(map[string]interface{}) return &Config{mapping} } // GetString fetches the string value by indicating the key. // It returns a ValueTypeError if the value is not a sring. -func (config *Config) GetString(key string, defaultValue interface{}) (string, error) { +func (config *Config) GetString(key string, defaultValue string) (string, error) { value, err := config.Get(key, defaultValue) if err != nil { - return "", err + return defaultValue, err } if result, ok := value.(string); ok { return result, nil } - return "", &ValueTypeError{key: key, value: value, message: "Value is not a string."} + return defaultValue, &ValueTypeError{key: key, value: value, message: "Value is not a string."} } // GetInt fetches the int value by indicating the key. // It returns a ValueTypeError if the value is not a sring. -func (config *Config) GetInt(key string, defaultValue interface{}) (int, error) { +func (config *Config) GetInt(key string, defaultValue int) (int, error) { value, err := config.Get(key, defaultValue) if err != nil { - return 0, err + return defaultValue, err } if result, ok := value.(int); ok { return result, nil } - return 0, &ValueTypeError{key: key, value: value, message: "Value is not an integer."} + return defaultValue, &ValueTypeError{key: key, value: value, message: "Value is not an integer."} } // GetBool fetches the bool value by indicating the key. // It returns a ValueTypeError if the value is not a sring. -func (config *Config) GetBool(key string, defaultValue interface{}) (bool, error) { +func (config *Config) GetBool(key string, defaultValue bool) (bool, error) { value, err := config.Get(key, defaultValue) if err != nil { - return false, err + return defaultValue, err } if result, ok := value.(bool); ok { return result, nil } - return false, &ValueTypeError{key: key, value: value, message: "Value is not an bool."} + return defaultValue, &ValueTypeError{key: key, value: value, message: "Value is not an bool."} } // GetFloat fetches the float value by indicating the key. // It returns a ValueTypeError if the value is not a sring. -func (config *Config) GetFloat(key string, defaultValue interface{}) (float64, error) { +func (config *Config) GetFloat(key string, defaultValue float64) (float64, error) { value, err := config.Get(key, defaultValue) if err != nil { - return 0, err + return defaultValue, err } if result, ok := value.(float64); ok { return result, nil } - return 0, &ValueTypeError{key: key, value: value, message: "Value is not a float."} + return defaultValue, &ValueTypeError{key: key, value: value, message: "Value is not a float."} } // Set is used to set the value by indicating the key. @@ -151,7 +151,7 @@ func (config *Config) Get(key string, defaultValue interface{}) (interface{}, er if value, exists := mapping[item]; exists { tmp = value } else if defaultValue != nil { - return defaultValue, nil + return nil, &KeyError{key: path.Join(append(keys[:i], item)...)} } else { return nil, &KeyError{key: path.Join(append(keys[:i], item)...)} } diff --git a/config_test.go b/config_test.go index c118c05..8bfe33e 100644 --- a/config_test.go +++ b/config_test.go @@ -22,8 +22,7 @@ func TestConfig(t *testing.T) { defaultValue := "None" for _, c := range cases { - app := New() - config := NewConfig(app) + config := NewConfig() config.Set(c.key, c.value) value, err := config.Get(c.key, defaultValue) @@ -46,8 +45,7 @@ func TestConfigWithMultipleEntires(t *testing.T) { {"foo2", "bar4"}, } - app := New() - config := NewConfig(app) + config := NewConfig() for _, c := range settings { config.Set(c.key, c.value) @@ -80,3 +78,91 @@ func TestFromJSON(t *testing.T) { t.Errorf("expected value to be abc but it was %v", value) } } + +func TestGetStringException(t *testing.T) { + defaultValue := "None" + + config := NewConfig() + config.Set("foo", 123) + val, err := config.GetString("foo", defaultValue) + if err == nil { + t.Errorf("Should have raised an type error when getting a non-string value by GetString.") + } + if val != defaultValue { + t.Errorf("Should have used the default value when raising an error.") + } + + val, err = config.GetString("bar", defaultValue) + if err == nil { + t.Errorf("Should have raised an type error when getting a non-existed value by GetString.") + } + if val != defaultValue { + t.Errorf("Should have used the default value when raising an error.") + } +} + +func TestGetIntegerException(t *testing.T) { + defaultValue := 123 + + config := NewConfig() + config.Set("foo", "bar") + val, err := config.GetInt("foo", defaultValue) + if err == nil { + t.Errorf("Should have raised an type error when getting a non-string value by GetString.") + } + if val != defaultValue { + t.Errorf("Should have used the default value when raising an error.") + } + + val, err = config.GetInt("bar", defaultValue) + if err == nil { + t.Errorf("Should have raised an type error when getting a non-existed value by GetString.") + } + if val != defaultValue { + t.Errorf("Should have used the default value when raising an error.") + } +} + +func TestGetBoolException(t *testing.T) { + defaultValue := false + + config := NewConfig() + config.Set("foo", "bar") + val, err := config.GetBool("foo", defaultValue) + if err == nil { + t.Errorf("Should have raised an type error when getting a non-string value by GetString.") + } + if val != defaultValue { + t.Errorf("Should have used the default value when raising an error.") + } + + val, err = config.GetBool("bar", defaultValue) + if err == nil { + t.Errorf("Should have raised an type error when getting a non-existed value by GetString.") + } + if val != defaultValue { + t.Errorf("Should have used the default value when raising an error.") + } +} + +func TestGetFloat64Exception(t *testing.T) { + defaultValue := 0.5 + + config := NewConfig() + config.Set("foo", "bar") + val, err := config.GetFloat("foo", defaultValue) + if err == nil { + t.Errorf("Should have raised an type error when getting a non-string value by GetString.") + } + if val != defaultValue { + t.Errorf("Should have used the default value when raising an error.") + } + + val, err = config.GetFloat("bar", defaultValue) + if err == nil { + t.Errorf("Should have raised an type error when getting a non-existed value by GetString.") + } + if val != defaultValue { + t.Errorf("Should have used the default value when raising an error.") + } +} From b4a20a3782dd86ffb9f0930af91a646defdedf62 Mon Sep 17 00:00:00 2001 From: dinever Date: Fri, 22 Apr 2016 09:15:16 -0400 Subject: [PATCH 15/15] [test] Improve test coverage for Router and Session --- app.go | 2 +- router_test.go | 2 +- session_test.go | 16 ++++++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/app.go b/app.go index 8ab3740..e5251df 100644 --- a/app.go +++ b/app.go @@ -48,7 +48,7 @@ func New() *Application { app.router = newRouter() app.staticRouter = make(map[string][]string) app.View = NewView() - app.Config = NewConfig(app) + app.Config = NewConfig() // debug, _ := app.Config.GetBool("debug", false) app.errorHandler = make(map[int]ErrorHandlerFunc) app.MiddlewareChain = NewChain() diff --git a/router_test.go b/router_test.go index 4c6ccaa..ee85174 100644 --- a/router_test.go +++ b/router_test.go @@ -110,7 +110,7 @@ func TestIncorrectPath(t *testing.T) { func TestPathNotFound(t *testing.T) { path := []struct { method, path, incomingMethod, incomingPath string - } { + }{ {"GET", "/users/name/", "GET", "/users/name/dinever/"}, {"GET", "/dinever/repo/", "POST", "/dinever/repo/"}, } diff --git a/session_test.go b/session_test.go index 210e41d..509a21e 100644 --- a/session_test.go +++ b/session_test.go @@ -73,3 +73,19 @@ func TestMemorySessionNotExpire(t *testing.T) { t.Errorf("Falsely recycled non-expired sessions.") } } + +func TestMemorySessionCount(t *testing.T) { + mgr := NewMemorySessionManager() + for i := 0; i < 100; i++ { + sid, _ := mgr.sessionID() + s := MemorySession{sid: sid, data: make(map[string]interface{}), createdAt: time.Now().AddDate(0, 0, -1)} + mgr.sessions[sid] = &s + } + if mgr.Count() != 100 { + t.Errorf("Could not correctly get session count: %v != %v", mgr.Count(), 100) + } + mgr.GarbageCollection() + if mgr.Count() != 0 { + t.Errorf("Could not correctly get session count after GC: %v != %v", mgr.Count(), 0) + } +}