From 120ba540f84c5b0e373200da7ee4dc74bb49397b Mon Sep 17 00:00:00 2001 From: tigerwill90 Date: Mon, 21 Oct 2024 15:00:59 +0200 Subject: [PATCH 01/15] feat: wip infix wildcard --- error.go | 5 +- fox.go | 29 +++-- fox_test.go | 22 ++++ node.go | 98 +++++++++-------- tree.go | 298 ++++++++++++++++++++++++++++++++++++++-------------- 5 files changed, 313 insertions(+), 139 deletions(-) diff --git a/error.go b/error.go index 6341501..28adcee 100644 --- a/error.go +++ b/error.go @@ -32,10 +32,7 @@ type RouteConflictError struct { isUpdate bool } -func newConflictErr(method, path, catchAllKey string, matched []string) *RouteConflictError { - if catchAllKey != "" { - path += "*{" + catchAllKey + "}" - } +func newConflictErr(method, path string, matched []string) *RouteConflictError { return &RouteConflictError{ Method: method, Path: path, diff --git a/fox.go b/fox.go index 6493201..c21e182 100644 --- a/fox.go +++ b/fox.go @@ -188,6 +188,9 @@ func (fox *Router) NewTree() *Tree { return tree.allocateContext() }, } + tree.np = sync.Pool{ + New: func() any { return tree.allocateNode() }, + } return tree } @@ -343,11 +346,11 @@ func (fox *Router) ServeHTTP(w http.ResponseWriter, r *http.Request) { nds := *tree.nodes.Load() index := findRootNode(r.Method, nds) - if index < 0 { + if index < 0 || len(nds[index].children) == 0 { goto NoMethodFallback } - n, tsr = tree.lookup(nds[index], target, c, false) + n, tsr = tree.lookup(nds[index].children[0].Load(), target, c, false) if !tsr && n != nil { c.route = n.route c.tsr = tsr @@ -404,11 +407,13 @@ NoMethodFallback: } else { // Since different method and route may match (e.g. GET /foo/bar & POST /foo/{name}), we cannot set the path and params. for i := 0; i < len(nds); i++ { - if n, tsr := tree.lookup(nds[i], target, c, true); n != nil && (!tsr || n.route.ignoreTrailingSlash) { - if sb.Len() > 0 { - sb.WriteString(", ") + if len(nds[i].children) > 0 { + if n, tsr := tree.lookup(nds[i].children[0].Load(), target, c, true); n != nil && (!tsr || n.route.ignoreTrailingSlash) { + if sb.Len() > 0 { + sb.WriteString(", ") + } + sb.WriteString(nds[i].key) } - sb.WriteString(nds[i].key) } } } @@ -425,15 +430,18 @@ NoMethodFallback: var sb strings.Builder for i := 0; i < len(nds); i++ { if nds[i].key != r.Method { - if n, tsr := tree.lookup(nds[i], target, c, true); n != nil && (!tsr || n.route.ignoreTrailingSlash) { - if sb.Len() > 0 { - sb.WriteString(", ") + if len(nds[i].children) > 0 { + if n, tsr := tree.lookup(nds[i], target, c, true); n != nil && (!tsr || n.route.ignoreTrailingSlash) { + if sb.Len() > 0 { + sb.WriteString(", ") + } + sb.WriteString(nds[i].key) } - sb.WriteString(nds[i].key) } } } if sb.Len() > 0 { + // TODO maybe should add OPTIONS ? w.Header().Set(HeaderAllow, sb.String()) c.scope = NoMethodHandler fox.noMethod(c) @@ -615,6 +623,7 @@ func parseRoute(path string) (string, string, int, error) { func getRouteConflict(n *node) []string { routes := make([]string, 0) + // TODO, we have to revise that if n.isCatchAll() { routes = append(routes, n.route.path) return routes diff --git a/fox_test.go b/fox_test.go index e9e9283..dd5396e 100644 --- a/fox_test.go +++ b/fox_test.go @@ -3525,3 +3525,25 @@ func ExampleTree_Has() { func onlyError[T any](_ T, err error) error { return err } + +func TestX(t *testing.T) { + f := New(WithIgnoreTrailingSlash(true)) + tree := f.Tree() + // require.NoError(t, tree.insert("GET", "/foo/*{args}", "args", 1, &Route{path: "/foo/*{args}"})) + // require.NoError(t, tree.insert("GET", "/foo/", "", 1, &Route{path: "/foo/"})) + require.NoError(t, tree.insert("GET", "/foo/*{args}", "", 1, &Route{path: "/foo/*{args}"})) + // require.NoError(t, tree.insert("GET", "/{x}/a/b/c/trackk", "", 1, &Route{path: "/{x}/a/b/c/trackk"})) + + nds := *tree.nodes.Load() + fmt.Println(nds[0]) + c := newTestContextTree(tree) + n, tsr := tree.lookup(nds[0].children[0].Load(), "/foo/", c, false) + fmt.Println("yolo", n, tsr, c.params) + // TODO tests + // - tsr priority rollback + // - wildcard suffix & wildcard infix + // - named parameter & wildcard infix + // - static & wildcard infix + // - empty infix wildcard + // current.key[current.params[paramKeyCnt].end:] (OK) +} diff --git a/node.go b/node.go index 4d8e1bf..aa857e6 100644 --- a/node.go +++ b/node.go @@ -18,12 +18,9 @@ type node struct { route *Route // key represent a segment of a route which share a common prefix with it parent. + // Once assigned, key is immutable. key string - // Catch all key registered to retrieve this node parameter. - // Once assigned, catchAllKey is immutable. - catchAllKey string - // First char of each outgoing edges from this node sorted in ascending order. // Once assigned, this is a read only slice. It allows to lazily traverse the // tree without the extra cost of atomic load operation. @@ -37,37 +34,43 @@ type node struct { params []param // The index of a paramChild if any, -1 if none (per rules, only one paramChildren is allowed). - paramChildIndex int + // Once assigned, paramChildIndex is immutable. + paramChildIndex int // TODO uint32 + // The index of a wildcardChild if any, -1 if none (per rules, only one wildcardChild is allowed). + // Once assigned, wildcardChildIndex is immutable. + wildcardChildIndex int } -func newNode(key string, route *Route, children []*node, catchAllKey string) *node { +func newNode(key string, route *Route, children []*node) *node { slices.SortFunc(children, func(a, b *node) int { return cmp.Compare(a.key, b.key) }) nds := make([]atomic.Pointer[node], len(children)) childKeys := make([]byte, len(children)) - childIndex := -1 + paramChildIndex := -1 + wildcardChildIndex := -1 for i := range children { assertNotNil(children[i]) childKeys[i] = children[i].key[0] nds[i].Store(children[i]) if strings.HasPrefix(children[i].key, "{") { - childIndex = i + paramChildIndex = i + } else if strings.HasPrefix(children[i].key, "*") { + wildcardChildIndex = i } } - - return newNodeFromRef(key, route, nds, childKeys, catchAllKey, childIndex) + return newNodeFromRef(key, route, nds, childKeys, paramChildIndex, wildcardChildIndex) } -func newNodeFromRef(key string, route *Route, children []atomic.Pointer[node], childKeys []byte, catchAllKey string, childIndex int) *node { +func newNodeFromRef(key string, route *Route, children []atomic.Pointer[node], childKeys []byte, paramChildIndex, wildcardChildIndex int) *node { return &node{ - key: key, - childKeys: childKeys, - children: children, - route: route, - catchAllKey: catchAllKey, - paramChildIndex: childIndex, - params: parseWildcard(key), + key: key, + childKeys: childKeys, + children: children, + route: route, + paramChildIndex: paramChildIndex, + wildcardChildIndex: wildcardChildIndex, + params: parseWildcard(key), } } @@ -76,7 +79,7 @@ func (n *node) isLeaf() bool { } func (n *node) isCatchAll() bool { - return n.catchAllKey != "" + return n.wildcardChildIndex >= 0 } func (n *node) hasWildcard() bool { @@ -204,6 +207,7 @@ func (n *node) string(space int) string { } + // TODO we have to revise that if n.isCatchAll() { sb.WriteString(" [catchAll]") } @@ -244,10 +248,10 @@ func (n *skippedNodes) pop() skippedNode { } type skippedNode struct { - n *node - pathIndex int - paramCnt uint32 - seen bool + n *node + pathIndex int + paramCnt uint32 + childIndex int } func appendCatchAll(path, catchAllKey string) string { @@ -263,8 +267,10 @@ func appendCatchAll(path, catchAllKey string) string { // param represents a parsed parameter and its end position in the path. type param struct { key string - end int // -1 if end with {a}, else pos of the next char - // catchAll bool + // -1 if end with {a}, else pos of the next char. Note that since infix wildcard are always followed + // by a static segment, pos should be always > 0 + end int + catchAll bool } func parseWildcard(segment string) []param { @@ -289,28 +295,28 @@ func parseWildcard(segment string) []param { state = stateDefault } i++ - //case stateCatchAll: - //if segment[i] == '}' { - // end := -1 - // if len(segment[i+1:]) > 0 { - // end = i + 1 - // } - // params = append(params, param{ - // key: segment[start:i], - // end: end, - // catchAll: true, - // }) - // start = 0 - // state = stateDefault - //} - //i++ + case stateCatchAll: + if segment[i] == '}' { + end := -1 + if len(segment[i+1:]) > 0 { + end = i + 1 + } + params = append(params, param{ + key: segment[start:i], + end: end, + catchAll: true, + }) + start = 0 + state = stateDefault + } + i++ default: - // if segment[i] == '*' { - // state = stateCatchAll - // i += 2 - // start = i - // continue - //} + if segment[i] == '*' { + state = stateCatchAll + i += 2 + start = i + continue + } if segment[i] == '{' { state = stateParam diff --git a/tree.go b/tree.go index 666cce8..834f1d2 100644 --- a/tree.go +++ b/tree.go @@ -32,6 +32,7 @@ import ( // defer fox.Tree().Unlock() type Tree struct { ctx sync.Pool + np sync.Pool nodes atomic.Pointer[[]*node] fox *Router sync.Mutex @@ -218,7 +219,7 @@ func (t *Tree) Iter() Iter { // Insert is not safe for concurrent use. The path must start by '/' and it's not validated. Use // parseRoute before. -func (t *Tree) insert(method, path, catchAllKey string, paramsN uint32, route *Route) error { +func (t *Tree) insert(method, path, _ string, paramsN uint32, route *Route) error { // Note that we need a consistent view of the tree during the patching so search must imperatively be locked. if !t.race.CompareAndSwap(0, 1) { panic(ErrConcurrentAccess) @@ -235,7 +236,7 @@ func (t *Tree) insert(method, path, catchAllKey string, paramsN uint32, route *R rootNode = nds[index] } - isCatchAll := catchAllKey != "" + // isCatchAll := catchAllKey != "" result := t.search(rootNode, path) switch result.classify() { @@ -246,15 +247,23 @@ func (t *Tree) insert(method, path, catchAllKey string, paramsN uint32, route *R // └── am // Create a new node from "st" reference and update the "te" (parent) reference to "st" node. if result.matched.isLeaf() { - if result.matched.isCatchAll() && isCatchAll { - return newConflictErr(method, path, catchAllKey, getRouteConflict(result.matched)) - } + // TODO it's probably no needed anymore since node key contain now also the catch-all part + /* if result.matched.isCatchAll() && isCatchAll { + return newConflictErr(method, path, getRouteConflict(result.matched)) + }*/ return fmt.Errorf("%w: new route %s %s conflict with %s", ErrRouteExist, method, route.path, result.matched.route.path) } // We are updating an existing node. We only need to create a new node from // the matched one with the updated/added value (handler and wildcard). - n := newNodeFromRef(result.matched.key, route, result.matched.children, result.matched.childKeys, catchAllKey, result.matched.paramChildIndex) + n := newNodeFromRef( + result.matched.key, + route, + result.matched.children, + result.matched.childKeys, + result.matched.paramChildIndex, + result.matched.wildcardChildIndex, + ) t.updateMaxParams(paramsN) result.p.updateEdge(n) @@ -284,15 +293,14 @@ func (t *Tree) insert(method, path, catchAllKey string, paramsN uint32, route *R result.matched.route, result.matched.children, result.matched.childKeys, - result.matched.catchAllKey, result.matched.paramChildIndex, + result.matched.wildcardChildIndex, ) parent := newNode( cPrefix, route, []*node{child}, - catchAllKey, ) t.updateMaxParams(paramsN) @@ -316,14 +324,13 @@ func (t *Tree) insert(method, path, catchAllKey string, paramsN uint32, route *R keySuffix := path[result.charsMatched:] // No children, so no paramChild - child := newNode(keySuffix, route, nil, catchAllKey) + child := newNode(keySuffix, route, nil) edges := result.matched.getEdgesShallowCopy() edges = append(edges, child) n := newNode( result.matched.key, result.matched.route, edges, - result.matched.catchAllKey, ) t.updateMaxDepth(result.depth + 1) @@ -359,32 +366,32 @@ func (t *Tree) insert(method, path, catchAllKey string, paramsN uint32, route *R keyCharsFromStartOfNodeFound := path[result.charsMatched-result.charsMatchedInNodeFound:] cPrefix := commonPrefix(keyCharsFromStartOfNodeFound, result.matched.key) - // Rule: a node with {param} has no child or has a separator before the end of the key + // Rule: a node with {param} or *{wildcard} has no child or has a separator before the end of the key for i := len(cPrefix) - 1; i >= 0; i-- { if cPrefix[i] == '/' { break } - if cPrefix[i] == '{' { - return newConflictErr(method, path, catchAllKey, getRouteConflict(result.matched)) + if cPrefix[i] == '{' || cPrefix[i] == '*' { + return newConflictErr(method, path, getRouteConflict(result.matched)) } } suffixFromExistingEdge := strings.TrimPrefix(result.matched.key, cPrefix) keySuffix := path[result.charsMatched:] - // No children, so no paramChild - n1 := newNodeFromRef(keySuffix, route, nil, nil, catchAllKey, -1) // inserted node + // No children, so no paramChild or wildcardChild + n1 := newNodeFromRef(keySuffix, route, nil, nil, -1, -1) // inserted node n2 := newNodeFromRef( suffixFromExistingEdge, result.matched.route, result.matched.children, result.matched.childKeys, - result.matched.catchAllKey, result.matched.paramChildIndex, + result.matched.wildcardChildIndex, ) // previous matched node // n3 children never start with a param - n3 := newNode(cPrefix, nil, []*node{n1, n2}, "") // intermediary node + n3 := newNode(cPrefix, nil, []*node{n1, n2}) // intermediary node t.updateMaxDepth(result.depth + 1) t.updateMaxParams(paramsN) @@ -397,7 +404,7 @@ func (t *Tree) insert(method, path, catchAllKey string, paramsN uint32, route *R } // update is not safe for concurrent use. -func (t *Tree) update(method string, path, catchAllKey string, route *Route) error { +func (t *Tree) update(method string, path, _ string, route *Route) error { // Note that we need a consistent view of the tree during the patching so search must imperatively be locked. if !t.race.CompareAndSwap(0, 1) { panic(ErrConcurrentAccess) @@ -415,11 +422,12 @@ func (t *Tree) update(method string, path, catchAllKey string, route *Route) err return fmt.Errorf("%w: route %s %s is not registered", ErrRouteNotFound, method, path) } - if catchAllKey != result.matched.catchAllKey { + // TODO this is probably not needed any more + /* if catchAllKey != result.matched.catchAllKey { err := newConflictErr(method, path, catchAllKey, []string{result.matched.route.path}) err.isUpdate = true return err - } + }*/ // We are updating an existing node (could be a leaf or not). We only need to create a new node from // the matched one with the updated/added value (handler and wildcard). @@ -428,15 +436,15 @@ func (t *Tree) update(method string, path, catchAllKey string, route *Route) err route, result.matched.children, result.matched.childKeys, - catchAllKey, result.matched.paramChildIndex, + result.matched.wildcardChildIndex, ) result.p.updateEdge(n) return nil } // remove is not safe for concurrent use. -func (t *Tree) remove(method, path, catchAllKey string) bool { +func (t *Tree) remove(method, path, _ string) bool { // Note that we need a consistent view of the tree during the patching so search must imperatively be locked. if !t.race.CompareAndSwap(0, 1) { panic(ErrConcurrentAccess) @@ -450,7 +458,7 @@ func (t *Tree) remove(method, path, catchAllKey string) bool { } result := t.search(nds[index], path) - if result.classify() != exactMatch || catchAllKey != result.matched.catchAllKey { + if result.classify() != exactMatch /*|| (TODO probably not needed anymore) catchAllKey != result.matched.catchAllKey */ { return false } @@ -466,8 +474,8 @@ func (t *Tree) remove(method, path, catchAllKey string) bool { nil, result.matched.children, result.matched.childKeys, - "", result.matched.paramChildIndex, + result.matched.wildcardChildIndex, ) result.p.updateEdge(n) return true @@ -481,8 +489,8 @@ func (t *Tree) remove(method, path, catchAllKey string) bool { child.route, child.children, child.childKeys, - child.catchAllKey, child.paramChildIndex, + child.wildcardChildIndex, ) result.p.updateEdge(n) return true @@ -509,15 +517,14 @@ func (t *Tree) remove(method, path, catchAllKey string) bool { child.route, child.children, child.childKeys, - child.catchAllKey, child.paramChildIndex, + child.wildcardChildIndex, ) } else { parent = newNode( result.p.key, result.p.route, parentEdges, - result.p.catchAllKey, ) } @@ -541,9 +548,9 @@ const ( ) func (t *Tree) lookup(rootNode *node, path string, c *cTx, lazy bool) (n *node, tsr bool) { - if len(rootNode.children) == 0 { + /* if len(rootNode.children) == 0 { return nil, false - } + }*/ var ( charsMatched int @@ -553,7 +560,8 @@ func (t *Tree) lookup(rootNode *node, path string, c *cTx, lazy bool) (n *node, parent *node ) - current := rootNode.children[0].Load() + // current := rootNode.children[0].Load() + current := rootNode *c.skipNds = (*c.skipNds)[:0] Walk: @@ -564,7 +572,12 @@ Walk: break } - if current.key[i] != path[charsMatched] || path[charsMatched] == bracketDelim { + x := string(current.key[i]) + y := string(path[charsMatched]) + _ = x + _ = y + + if current.key[i] != path[charsMatched] || path[charsMatched] == bracketDelim || path[charsMatched] == '*' { if current.key[i] == '{' { startPath := charsMatched idx := strings.IndexByte(path[charsMatched:], slashDelim) @@ -598,6 +611,105 @@ Walk: continue } + if current.key[i] == '*' { + // | current.params[paramKeyCnt].end (10) + // key: foo/*{bar}/ => 10 - 5 = 5 => i+=idx set i to '/' + // | charsMatchedInNodeFound (5) + idx := current.params[paramKeyCnt].end - charsMatchedInNodeFound + var interNode *node + if idx >= 0 { + // -1 since on the next incrementation, if any, 'i' are going to be incremented + // i += idx - 1 + charsMatchedInNodeFound += idx + + interNode = t.np.Get().(*node) + interNode.params = interNode.params[:0] + interNode = &node{ + route: current.route, + key: current.key[current.params[paramKeyCnt].end:], + childKeys: current.childKeys, + children: current.children, + paramChildIndex: current.paramChildIndex, + wildcardChildIndex: current.wildcardChildIndex, + } + for _, ps := range current.params[paramKeyCnt+1:] { // paramKeyCnt+1 is safe since we have at least the current infix wildcard in params + interNode.params = append(interNode.params, param{ + key: ps.key, + // end is relative to the original key, so we need to adjust the position relative to + // the new intermediary node. + end: ps.end - current.params[paramKeyCnt].end, + catchAll: ps.catchAll, + }) + } + + } else if len(current.children) > 0 { + // Infix catch all + interNode = current.get(0) + // -1 since on the next incrementation, if any, 'i' are going to be incremented + // i += len(current.key[charsMatchedInNodeFound:]) - 1 + charsMatchedInNodeFound += len(current.key[charsMatchedInNodeFound:]) + } else { + // We are in an ending catch all node with no child, so it's a direct match + if !lazy { + *c.params = append(*c.params, Param{Key: current.params[paramKeyCnt].key, Value: path[charsMatched:]}) + } + return current, false + } + + newCtx := t.ctx.Get().(*cTx) + startPath := charsMatched + for { + idx := strings.IndexByte(path[charsMatched:], slashDelim) + // idx >= 0, we have a next segment to evaluate + if idx >= 0 { + *newCtx.params = (*newCtx.params)[:0] + charsMatched += idx + x := path[charsMatched:] + _ = x + newNode, newTsr := t.lookup(interNode, path[charsMatched:], newCtx, false) + if newNode != nil { + t.np.Put(interNode) + // Reminder: we have to deal with tsrParams + if tsr && newTsr { + // TODO we have tsr match, and lookup return node with tsr, caller win because most specific + // We need to test tsr behaviour extensively + t.ctx.Put(newCtx) + break + } + + if !lazy { + *c.params = append(*c.params, Param{Key: current.params[paramKeyCnt].key, Value: path[startPath:charsMatched]}) + *c.params = append(*c.params, *newCtx.params...) + } + t.ctx.Put(newCtx) + return newNode, newTsr + } + charsMatched++ + // Try with next segment + continue + } + + t.ctx.Put(newCtx) + t.np.Put(interNode) + + // We are also in an ending catch all, and this is the most specific path + if current.params[paramKeyCnt].end == -1 { + if !lazy { + *c.params = append(*c.params, Param{Key: current.params[paramKeyCnt].key, Value: path[startPath:]}) + } + return current, false + } + + // char matched need to be re ajusted with probabley len(path + charsMatched += len(path[charsMatched:]) + // and this point len(path) == charsMatched + ctrl := len(path) == charsMatched + _ = ctrl + + break Walk + } + } + break Walk } @@ -615,28 +727,43 @@ Walk: } } - // Only one node which is a child param, load it directly and go deeper + // No next static segment found, but maybe some params or wildcard child if idx < 0 { - if current.paramChildIndex < 0 { - break - } + // We have at least a param child which is has higher priority that catch-all + if current.paramChildIndex >= 0 { + // We have also a wildcard child, save it for later evaluation + if current.wildcardChildIndex >= 0 { + *c.skipNds = append(*c.skipNds, skippedNode{current, charsMatched, paramCnt, current.wildcardChildIndex}) + } - // The node is also a catch-all, save it as the last fallback. - if current.catchAllKey != "" { - *c.skipNds = append(*c.skipNds, skippedNode{current, charsMatched, paramCnt, true}) + // Go deeper + idx = current.paramChildIndex + parent = current + current = current.children[idx].Load() + paramKeyCnt = 0 + continue + } + if current.wildcardChildIndex >= 0 { + // We have a wildcard child, go deeper + idx = current.wildcardChildIndex + parent = current + current = current.children[idx].Load() + paramKeyCnt = 0 + continue } - idx = current.paramChildIndex - parent = current - current = current.children[idx].Load() - paramKeyCnt = 0 - continue + // We have nothing more to evaluate + break } - // Save the node if we need to evaluate the child param or catch-all later - if current.paramChildIndex >= 0 || current.catchAllKey != "" { - *c.skipNds = append(*c.skipNds, skippedNode{current, charsMatched, paramCnt, false}) + // Here we have a next static segment and possibly wildcard children, so we save them for late evaluation if needed. + if current.paramChildIndex >= 0 { + *c.skipNds = append(*c.skipNds, skippedNode{current, charsMatched, paramCnt, current.paramChildIndex}) + } + if current.wildcardChildIndex >= 0 { + *c.skipNds = append(*c.skipNds, skippedNode{current, charsMatched, paramCnt, current.wildcardChildIndex}) } + parent = current current = current.children[idx].Load() paramKeyCnt = 0 @@ -686,16 +813,21 @@ Walk: if charsMatched == len(path) { if charsMatchedInNodeFound == len(current.key) { // Exact match, note that if we match a catch-all node - if !lazy && current.catchAllKey != "" { - *c.params = append(*c.params, Param{Key: current.catchAllKey, Value: path[charsMatched:]}) - // Exact match, tsr is always false - return current, false - } + /* if !lazy && current.catchAllKey != "" { + *c.params = append(*c.params, Param{Key: current.catchAllKey, Value: path[charsMatched:]}) + // Exact match, tsr is always false + return current, false + }*/ // Exact match, tsr is always false return current, false } if charsMatchedInNodeFound < len(current.key) { // Key end mid-edge + if !lazy && current.params[paramKeyCnt].catchAll { + *c.params = append(*c.params, Param{Key: current.params[paramKeyCnt].key, Value: path[charsMatched:]}) + // Exact match, tsr is always false + return current, false + } if !tsr { if strings.HasSuffix(path, "/") { // Tsr recommendation: remove the extra trailing slash (got an exact match) @@ -746,16 +878,16 @@ Walk: // Incomplete match to end of edge if charsMatched < len(path) && charsMatchedInNodeFound == len(current.key) { - if current.catchAllKey != "" { - if !lazy { - *c.params = append(*c.params, Param{Key: current.catchAllKey, Value: path[charsMatched:]}) - // Same as exact match, no tsr recommendation - return current, false - } - // Same as exact match, no tsr recommendation - return current, false - } - + /* if current.catchAllKey != "" { + if !lazy { + *c.params = append(*c.params, Param{Key: current.catchAllKey, Value: path[charsMatched:]}) + // Same as exact match, no tsr recommendation + return current, false + } + // Same as exact match, no tsr recommendation + return current, false + } + */ // Tsr recommendation: remove the extra trailing slash (got an exact match) if !tsr { remainingKeySuffix := path[charsMatched:] @@ -787,30 +919,30 @@ Walk: Backtrack: if hasSkpNds { skipped := c.skipNds.pop() - if skipped.n.paramChildIndex < 0 || skipped.seen { - // skipped is catch all - current = skipped.n - *c.params = (*c.params)[:skipped.paramCnt] + /* if skipped.n.paramChildIndex < 0 || skipped.seen { + // skipped is catch all + current = skipped.n + *c.params = (*c.params)[:skipped.paramCnt] - if !lazy { - *c.params = append(*c.params, Param{Key: current.catchAllKey, Value: path[skipped.pathIndex:]}) + if !lazy { + *c.params = append(*c.params, Param{Key: current.catchAllKey, Value: path[skipped.pathIndex:]}) + // Same as exact match, no tsr recommendation + return current, false + } // Same as exact match, no tsr recommendation return current, false - } - // Same as exact match, no tsr recommendation - return current, false - } + }*/ - // Could be a catch-all node with child param - // /foo/*{any} - // /foo/{bar} - // In this case we evaluate first the child param node and fall back to the catch-all. - if skipped.n.catchAllKey != "" { - *c.skipNds = append(*c.skipNds, skippedNode{skipped.n, skipped.pathIndex, skipped.paramCnt, true}) - } + /* // Could be a catch-all node with child param + // /foo/*{any} + // /foo/{bar} + // In this case we evaluate first the child param node and fall back to the catch-all. + if skipped.n.catchAllKey != "" { + *c.skipNds = append(*c.skipNds, skippedNode{skipped.n, skipped.pathIndex, skipped.paramCnt, true}) + }*/ parent = skipped.n - current = skipped.n.children[skipped.n.paramChildIndex].Load() + current = skipped.n.children[skipped.childIndex].Load() *c.params = (*c.params)[:skipped.paramCnt] charsMatched = skipped.pathIndex @@ -885,6 +1017,14 @@ func (t *Tree) allocateContext() *cTx { } } +// allocateNode is only used on the lookup phase when we have to dynamically create an intermediary node +// to recursively call Tree.lookup with a subtree. +func (t *Tree) allocateNode() *node { + return &node{ + params: make([]param, 0, t.maxParams.Load()), + } +} + // addRoot append a new root node to the tree. // Note: This function should be guarded by mutex. func (t *Tree) addRoot(n *node) { From e66165f7769403de374559f43fcddbc3563000d4 Mon Sep 17 00:00:00 2001 From: tigerwill90 Date: Mon, 21 Oct 2024 19:25:17 +0200 Subject: [PATCH 02/15] feat: wip test are passing --- fox.go | 90 ++++++++++-------- fox_test.go | 261 +++++++++++++++++++++++++++++++--------------------- iter.go | 4 +- node.go | 28 ++---- tree.go | 108 ++++++---------------- 5 files changed, 242 insertions(+), 249 deletions(-) diff --git a/fox.go b/fox.go index c21e182..d9f295e 100644 --- a/fox.go +++ b/fox.go @@ -6,6 +6,7 @@ package fox import ( "fmt" + "math" "net" "net/http" "path" @@ -431,7 +432,7 @@ NoMethodFallback: for i := 0; i < len(nds); i++ { if nds[i].key != r.Method { if len(nds[i].children) > 0 { - if n, tsr := tree.lookup(nds[i], target, c, true); n != nil && (!tsr || n.route.ignoreTrailingSlash) { + if n, tsr := tree.lookup(nds[i].children[0].Load(), target, c, true); n != nil && (!tsr || n.route.ignoreTrailingSlash) { if sb.Len() > 0 { sb.WriteString(", ") } @@ -544,16 +545,16 @@ const ( ) // parseRoute parse and validate the route in a single pass. -func parseRoute(path string) (string, string, int, error) { +func parseRoute(path string) (uint32, error) { if !strings.HasPrefix(path, "/") { - return "", "", -1, fmt.Errorf("%w: path must start with '/'", ErrInvalidRoute) + return 0, fmt.Errorf("%w: path must start with '/'", ErrInvalidRoute) } state := stateDefault previous := stateDefault - startCatchAll := 0 - paramCnt := 0 + paramCnt := uint32(0) + countStatic := 0 inParam := false i := 0 @@ -562,76 +563,87 @@ func parseRoute(path string) (string, string, int, error) { case stateParam: if path[i] == '}' { if !inParam { - return "", "", -1, fmt.Errorf("%w: missing parameter name between '{}'", ErrInvalidRoute) + return 0, fmt.Errorf("%w: missing parameter name between '{}'", ErrInvalidRoute) } inParam = false - if previous != stateCatchAll { - if i+1 < len(path) && path[i+1] != '/' { - return "", "", -1, fmt.Errorf("%w: unexpected character after '{param}'", ErrInvalidRoute) - } - } else { - if i+1 != len(path) { - return "", "", -1, fmt.Errorf("%w: catch-all '*{params}' are allowed only at the end of a route", ErrInvalidRoute) - } + if i+1 < len(path) && path[i+1] != '/' { + return 0, fmt.Errorf("%w: unexpected character after '{param}'", ErrInvalidRoute) } + + countStatic = 0 + previous = state state = stateDefault i++ continue } if path[i] == '/' || path[i] == '*' || path[i] == '{' { - return "", "", -1, fmt.Errorf("%w: unexpected character in '{params}'", ErrInvalidRoute) + return 0, fmt.Errorf("%w: unexpected character in '{params}'", ErrInvalidRoute) } inParam = true i++ - case stateCatchAll: - if path[i] != '{' { - return "", "", -1, fmt.Errorf("%w: unexpected character after '*' catch-all delimiter", ErrInvalidRoute) + if path[i] == '}' { + if !inParam { + return 0, fmt.Errorf("%w: missing parameter name between '*{}'", ErrInvalidRoute) + } + inParam = false + if i+1 < len(path) && path[i+1] != '/' { + return 0, fmt.Errorf("%w: unexpected character after '*{param}'", ErrInvalidRoute) + } + + if previous == stateCatchAll && countStatic <= 1 { + return 0, fmt.Errorf("%w: consecutive wildcard not allowed", ErrInvalidRoute) + } + + countStatic = 0 + previous = state + state = stateDefault + i++ + continue } - startCatchAll = i - previous = state - state = stateParam - i++ + if path[i] == '/' || path[i] == '*' || path[i] == '{' { + return 0, fmt.Errorf("%w: unexpected character in '*{params}'", ErrInvalidRoute) + } + inParam = true + i++ default: if path[i] == '{' { state = stateParam paramCnt++ } else if path[i] == '*' { state = stateCatchAll + i++ paramCnt++ + } else { + countStatic++ + } + + if paramCnt > math.MaxUint16 { + return 0, fmt.Errorf("%w: too many params (%d)", ErrInvalidRoute, paramCnt) } + i++ } } if state == stateParam { - return "", "", -1, fmt.Errorf("%w: unclosed '{params}'", ErrInvalidRoute) - } - if state == stateCatchAll { - return "", "", -1, fmt.Errorf("%w: missing '{params}' after '*' catch-all delimiter", ErrInvalidRoute) + return 0, fmt.Errorf("%w: unclosed '{params}'", ErrInvalidRoute) } - if startCatchAll > 0 { - return path[:startCatchAll-1], path[startCatchAll+1 : len(path)-1], paramCnt, nil + if state == stateCatchAll { + if path[len(path)-1] == '*' { + return 0, fmt.Errorf("%w: missing '{params}' after '*' catch-all delimiter", ErrInvalidRoute) + } + return 0, fmt.Errorf("%w: unclosed '*{params}'", ErrInvalidRoute) } - return path, "", paramCnt, nil + return paramCnt, nil } func getRouteConflict(n *node) []string { routes := make([]string, 0) - - // TODO, we have to revise that - if n.isCatchAll() { - routes = append(routes, n.route.path) - return routes - } - - if n.paramChildIndex >= 0 { - n = n.children[n.paramChildIndex].Load() - } it := newRawIterator(n) for it.hasNext() { routes = append(routes, it.current.route.path) diff --git a/fox_test.go b/fox_test.go index dd5396e..a4a4252 100644 --- a/fox_test.go +++ b/fox_test.go @@ -708,7 +708,7 @@ func TestRouterWildcard(t *testing.T) { }{ {"/github.com/etf1/*{repo}", "/github.com/etf1/mux"}, {"/github.com/johndoe/*{repo}", "/github.com/johndoe/buzz"}, - {"/foo/bar/*{args}", "/foo/bar/"}, + {"/foo/bar/*{args}", "/foo/bar/baz"}, {"/filepath/path=*{path}", "/filepath/path=/file.txt"}, } @@ -720,7 +720,7 @@ func TestRouterWildcard(t *testing.T) { req := httptest.NewRequest(http.MethodGet, route.key, nil) w := httptest.NewRecorder() r.ServeHTTP(w, req) - require.Equalf(t, http.StatusOK, w.Code, "route: key: %s, path: %s", route.path) + require.Equalf(t, http.StatusOK, w.Code, "route: key: %s, path: %s", route.key, route.path) assert.Equal(t, route.key, w.Body.String()) } } @@ -750,9 +750,9 @@ func TestRouteWithParams(t *testing.T) { nds := *tree.nodes.Load() for _, rte := range routes { c := newTestContextTree(tree) - n, tsr := tree.lookup(nds[0], rte, c, false) - require.NotNil(t, n) - require.NotNil(t, n.route) + n, tsr := tree.lookup(nds[0].children[0].Load(), rte, c, false) + require.NotNilf(t, n, "route: %s", rte) + require.NotNilf(t, n.route, "route: %s", rte) assert.False(t, tsr) assert.Equal(t, rte, n.route.path) } @@ -790,7 +790,7 @@ func TestRouteParamEmptySegment(t *testing.T) { t.Run(tc.name, func(t *testing.T) { nds := *tree.nodes.Load() c := newTestContextTree(tree) - n, tsr := tree.lookup(nds[0], tc.path, c, false) + n, tsr := tree.lookup(nds[0].children[0].Load(), tc.path, c, false) assert.Nil(t, n) assert.Empty(t, slices.Collect(c.Params())) assert.False(t, tsr) @@ -1168,7 +1168,7 @@ func TestOverlappingRoute(t *testing.T) { }, { name: "backtrack to most specific catch-all with an exact match", - path: "/foo/bar/", + path: "/foo/bar/x/y/z", routes: []string{ "/foo/{id_1}/abc", "/foo/{id_1}/*{args}", @@ -1182,7 +1182,25 @@ func TestOverlappingRoute(t *testing.T) { Value: "bar", }, { - Key: "args", + Key: "args", + Value: "x/y/z", + }, + }, + }, + { + name: "backtrack to most specific catch-all with an exact match", + path: "/foo/bar/", + routes: []string{ + "/foo/{id_1}/abc", + "/foo/{id_1}/*{args}", + "/foo/{id_1}/{id_2}", + "/foo/*{args}", + }, + wantMatch: "/foo/*{args}", + wantParams: Params{ + { + Key: "args", + Value: "bar/", }, }, }, @@ -1244,7 +1262,7 @@ func TestOverlappingRoute(t *testing.T) { nds := *tree.nodes.Load() c := newTestContextTree(tree) - n, tsr := tree.lookup(nds[0], tc.path, c, false) + n, tsr := tree.lookup(nds[0].children[0].Load(), tc.path, c, false) require.NotNil(t, n) require.NotNil(t, n.route) assert.False(t, tsr) @@ -1258,7 +1276,7 @@ func TestOverlappingRoute(t *testing.T) { // Test with lazy c = newTestContextTree(tree) - n, tsr = tree.lookup(nds[0], tc.path, c, true) + n, tsr = tree.lookup(nds[0].children[0].Load(), tc.path, c, true) require.NotNil(t, n) require.NotNil(t, n.route) assert.False(t, tsr) @@ -1286,7 +1304,7 @@ func TestInsertConflict(t *testing.T) { }{ {path: "/john/*{x}", wantErr: nil, wantMatch: nil}, {path: "/john/*{y}", wantErr: ErrRouteConflict, wantMatch: []string{"/john/*{x}"}}, - {path: "/john/", wantErr: ErrRouteExist, wantMatch: nil}, + {path: "/john/", wantErr: nil, wantMatch: nil}, {path: "/foo/baz", wantErr: nil, wantMatch: nil}, {path: "/foo/bar", wantErr: nil, wantMatch: nil}, {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, @@ -1296,7 +1314,7 @@ func TestInsertConflict(t *testing.T) { {path: "/avengers/{id}/foo", wantErr: nil, wantMatch: nil}, {path: "/avengers/*{args}", wantErr: nil, wantMatch: nil}, {path: "/fox/", wantErr: nil, wantMatch: nil}, - {path: "/fox/*{args}", wantErr: ErrRouteExist, wantMatch: nil}, + {path: "/fox/*{args}", wantErr: nil, wantMatch: nil}, }, }, { @@ -1341,12 +1359,18 @@ func TestInsertConflict(t *testing.T) { }{ {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, {path: "/foo/{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/foo/{id}"}}, + {path: "/gchq/*{id}", wantErr: nil, wantMatch: nil}, + {path: "/gchq/*{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/gchq/*{id}"}}, {path: "/foo{id}", wantErr: nil, wantMatch: nil}, {path: "/foo/a{id}", wantErr: nil, wantMatch: nil}, {path: "/avengers/{id}/bar", wantErr: nil, wantMatch: nil}, {path: "/avengers/{id}/baz", wantErr: nil, wantMatch: nil}, {path: "/avengers/{id}", wantErr: nil, wantMatch: nil}, {path: "/avengers/{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/avengers/{id}", "/avengers/{id}/bar", "/avengers/{id}/baz"}}, + {path: "/ironman/*{id}/bar", wantErr: nil, wantMatch: nil}, + {path: "/ironman/*{id}/baz", wantErr: nil, wantMatch: nil}, + {path: "/ironman/*{id}", wantErr: nil, wantMatch: nil}, + {path: "/ironman/*{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/ironman/*{id}", "/ironman/*{id}/bar", "/ironman/*{id}/baz"}}, }, }, } @@ -1359,7 +1383,7 @@ func TestInsertConflict(t *testing.T) { if err != nil { assert.Nil(t, r) } - assert.ErrorIs(t, err, rte.wantErr) + assert.ErrorIsf(t, err, rte.wantErr, "route: %s", rte.path) if cErr, ok := err.(*RouteConflictError); ok { assert.Equal(t, rte.wantMatch, cErr.Matched) } @@ -1398,21 +1422,21 @@ func TestUpdateConflict(t *testing.T) { name: "wildcard have different name", routes: []string{"/foo/bar", "/foo/*{args}"}, update: "/foo/*{all}", - wantErr: ErrRouteConflict, + wantErr: ErrRouteNotFound, wantMatch: []string{"/foo/*{args}"}, }, { name: "replacing non wildcard by wildcard", routes: []string{"/foo/bar", "/foo/"}, update: "/foo/*{all}", - wantErr: ErrRouteConflict, + wantErr: ErrRouteNotFound, wantMatch: []string{"/foo/"}, }, { name: "replacing wildcard by non wildcard", routes: []string{"/foo/bar", "/foo/*{args}"}, update: "/foo/", - wantErr: ErrRouteConflict, + wantErr: ErrRouteNotFound, wantMatch: []string{"/foo/*{args}"}, }, } @@ -1428,9 +1452,6 @@ func TestUpdateConflict(t *testing.T) { assert.Nil(t, r) } assert.ErrorIs(t, err, tc.wantErr) - if cErr, ok := err.(*RouteConflictError); ok { - assert.Equal(t, tc.wantMatch, cErr.Matched) - } }) } } @@ -1503,145 +1524,172 @@ func TestUpdateRoute(t *testing.T) { func TestParseRoute(t *testing.T) { cases := []struct { - wantErr error - name string - path string - wantCatchAllKey string - wantPath string - wantN int + wantErr error + name string + path string + wantN uint32 }{ { - name: "valid static route", - path: "/foo/bar", - wantPath: "/foo/bar", + name: "valid static route", + path: "/foo/bar", + }, + { + name: "valid catch all route", + path: "/foo/bar/*{arg}", + wantN: 1, + }, + { + name: "valid param route", + path: "/foo/bar/{baz}", + wantN: 1, + }, + { + name: "valid multi params route", + path: "/foo/{bar}/{baz}", + wantN: 2, }, { - name: "valid catch all route", - path: "/foo/bar/*{arg}", - wantN: 1, - wantCatchAllKey: "arg", - wantPath: "/foo/bar/", + name: "valid same params route", + path: "/foo/{bar}/{bar}", + wantN: 2, }, { - name: "valid param route", - path: "/foo/bar/{baz}", - wantN: 1, - wantPath: "/foo/bar/{baz}", + name: "valid multi params and catch all route", + path: "/foo/{bar}/{baz}/*{arg}", + wantN: 3, }, { - name: "valid multi params route", - path: "/foo/{bar}/{baz}", - wantN: 2, - wantPath: "/foo/{bar}/{baz}", + name: "valid inflight param", + path: "/foo/xyz:{bar}", + wantN: 1, }, { - name: "valid same params route", - path: "/foo/{bar}/{bar}", - wantN: 2, - wantPath: "/foo/{bar}/{bar}", + name: "valid inflight catchall", + path: "/foo/xyz:*{bar}", + wantN: 1, }, { - name: "valid multi params and catch all route", - path: "/foo/{bar}/{baz}/*{arg}", - wantN: 3, - wantCatchAllKey: "arg", - wantPath: "/foo/{bar}/{baz}/", + name: "valid multi inflight param and catch all", + path: "/foo/xyz:{bar}/abc:{bar}/*{arg}", + wantN: 3, }, { - name: "valid inflight param", - path: "/foo/xyz:{bar}", - wantN: 1, - wantPath: "/foo/xyz:{bar}", + name: "catch all with arg in the middle of the route", + path: "/foo/bar/*{bar}/baz", + wantN: 1, }, { - name: "valid inflight catchall", - path: "/foo/xyz:*{bar}", - wantN: 1, - wantPath: "/foo/xyz:", - wantCatchAllKey: "bar", + name: "multiple catch all suffix and inflight with arg in the middle of the route", + path: "/foo/bar/*{bar}/x*{args}/y/*{z}/{b}", + wantN: 4, }, { - name: "valid multi inflight param and catch all", - path: "/foo/xyz:{bar}/abc:{bar}/*{arg}", - wantN: 3, - wantCatchAllKey: "arg", - wantPath: "/foo/xyz:{bar}/abc:{bar}/", + name: "inflight catch all with arg in the middle of the route", + path: "/foo/bar/damn*{bar}/baz", + wantN: 1, + }, + { + name: "catch all with arg in the middle of the route and param after", + path: "/foo/bar/*{bar}/{baz}", + wantN: 2, }, { name: "missing prefix slash", path: "foo/bar", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "empty parameter", path: "/foo/bar{}", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "missing arguments name after catch all", path: "/foo/bar/*", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "missing arguments name after param", path: "/foo/bar/{", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "catch all in the middle of the route", path: "/foo/bar/*/baz", wantErr: ErrInvalidRoute, - wantN: -1, - }, - { - name: "catch all with arg in the middle of the route", - path: "/foo/bar/*{bar}/baz", - wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "unexpected character in param", path: "/foo/{{bar}", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "unexpected character in param", path: "/foo/{*bar}", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "in flight catch-all after param in one route segment", path: "/foo/{bar}*{baz}", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "multiple param in one route segment", path: "/foo/{bar}{baz}", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "in flight param after catch all", path: "/foo/*{args}{param}", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, + }, + { + name: "consecutive catch all with no slash", + path: "/foo/*{args}*{param}", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "consecutive catch all", + path: "/foo/*{args}/*{param}", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "consecutive catch all with inflight", + path: "/foo/ab*{args}/*{param}", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "unexpected char after inflight catch all", + path: "/foo/ab*{args}a", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "unexpected char after catch all", + path: "/foo/*{args}a", + wantErr: ErrInvalidRoute, + wantN: 0, }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - path, key, n, err := parseRoute(tc.path) + n, err := parseRoute(tc.path) require.ErrorIs(t, err, tc.wantErr) assert.Equal(t, tc.wantN, n) - assert.Equal(t, tc.wantCatchAllKey, key) - assert.Equal(t, tc.wantPath, path) }) } } @@ -1718,11 +1766,11 @@ func TestTree_LookupTsr(t *testing.T) { t.Run(tc.name, func(t *testing.T) { tree := New().Tree() for _, path := range tc.paths { - require.NoError(t, tree.insert(http.MethodGet, path, "", 0, tree.newRoute(path, emptyHandler))) + require.NoError(t, tree.insert(http.MethodGet, tree.newRoute(path, emptyHandler), 0)) } nds := *tree.nodes.Load() c := newTestContextTree(tree) - n, got := tree.lookup(nds[0], tc.key, c, true) + n, got := tree.lookup(nds[0].children[0].Load(), tc.key, c, true) assert.Equal(t, tc.want, got) if tc.want { require.NotNil(t, n) @@ -3067,11 +3115,11 @@ func TestFuzzInsertLookupParam(t *testing.T) { continue } path := fmt.Sprintf(routeFormat, s1, e1, s2, e2, e3) - if err := tree.insert(http.MethodGet, path, "", 3, tree.newRoute(path, emptyHandler)); err == nil { + if err := tree.insert(http.MethodGet, tree.newRoute(path, emptyHandler), 3); err == nil { nds := *tree.nodes.Load() c := newTestContextTree(tree) - n, tsr := tree.lookup(nds[0], fmt.Sprintf(reqFormat, s1, "xxxx", s2, "xxxx", "xxxx"), c, false) + n, tsr := tree.lookup(nds[0].children[0].Load(), fmt.Sprintf(reqFormat, s1, "xxxx", s2, "xxxx", "xxxx"), c, false) require.NotNil(t, n) require.NotNil(t, n.route) assert.False(t, tsr) @@ -3091,14 +3139,12 @@ func TestFuzzInsertNoPanics(t *testing.T) { f.Fuzz(&routes) for rte := range routes { - var catchAllKey string - f.Fuzz(&catchAllKey) if rte == "" { continue } require.NotPanicsf(t, func() { - _ = tree.insert(http.MethodGet, rte, catchAllKey, 0, tree.newRoute(appendCatchAll(rte, catchAllKey), emptyHandler)) - }, fmt.Sprintf("rte: %s, catch all: %s", rte, catchAllKey)) + _ = tree.insert(http.MethodGet, tree.newRoute(rte, emptyHandler), 0) + }, fmt.Sprintf("rte: %s", rte)) } } @@ -3119,7 +3165,7 @@ func TestFuzzInsertLookupUpdateAndDelete(t *testing.T) { for rte := range routes { path := "/" + rte - err := tree.insert(http.MethodGet, path, "", 0, tree.newRoute(path, emptyHandler)) + err := tree.insert(http.MethodGet, tree.newRoute(path, emptyHandler), 0) require.NoError(t, err) } @@ -3130,18 +3176,18 @@ func TestFuzzInsertLookupUpdateAndDelete(t *testing.T) { for rte := range routes { nds := *tree.nodes.Load() c := newTestContextTree(tree) - n, tsr := tree.lookup(nds[0], "/"+rte, c, true) + n, tsr := tree.lookup(nds[0].children[0].Load(), "/"+rte, c, true) require.NotNilf(t, n, "route /%s", rte) require.NotNilf(t, n.route, "route /%s", rte) require.Falsef(t, tsr, "tsr: %t", tsr) require.Truef(t, n.isLeaf(), "route /%s", rte) require.Equal(t, "/"+rte, n.route.path) path := "/" + rte - require.NoError(t, tree.update(http.MethodGet, path, "", tree.newRoute(path, emptyHandler))) + require.NoError(t, tree.update(http.MethodGet, tree.newRoute(path, emptyHandler))) } for rte := range routes { - deleted := tree.remove(http.MethodGet, "/"+rte, "") + deleted := tree.remove(http.MethodGet, "/"+rte) require.True(t, deleted) } @@ -3221,7 +3267,7 @@ func TestTree_RaceDetector(t *testing.T) { } wg.Done() }() - tree.insert(rte.method, rte.path, "", 0, &Route{path: rte.path}) + _ = tree.insert(rte.method, &Route{path: rte.path}, 0) }() go func() { @@ -3233,7 +3279,7 @@ func TestTree_RaceDetector(t *testing.T) { } wg.Done() }() - tree.update(rte.method, rte.path, "", &Route{path: rte.path}) + _ = tree.update(rte.method, &Route{path: rte.path}) }() go func() { @@ -3245,7 +3291,7 @@ func TestTree_RaceDetector(t *testing.T) { } wg.Done() }() - tree.remove(rte.method, rte.path, "") + tree.remove(rte.method, rte.path) }() } @@ -3343,7 +3389,7 @@ func TestNode_String(t *testing.T) { nds := *tree.nodes.Load() want := `path: GET - path: /foo/{bar}/ [catchAll] [leaf=/foo/{bar}/*{baz}] [bar (10)]` + path: /foo/{bar}/*{baz} [leaf=/foo/{bar}/*{baz}] [bar (10), baz (-1)]` assert.Equal(t, want, strings.TrimSuffix(nds[0].String(), "\n")) } @@ -3529,16 +3575,23 @@ func onlyError[T any](_ T, err error) error { func TestX(t *testing.T) { f := New(WithIgnoreTrailingSlash(true)) tree := f.Tree() - // require.NoError(t, tree.insert("GET", "/foo/*{args}", "args", 1, &Route{path: "/foo/*{args}"})) + + require.NoError(t, tree.insert("GET", &Route{path: "/foo/"}, 0)) + require.NoError(t, tree.insert("GET", &Route{path: "/foo/*{args}"}, 1)) + require.NoError(t, tree.insert("GET", &Route{path: "/foo/*{args}/bat"}, 1)) + require.NoError(t, tree.insert("GET", &Route{path: "/foo/{ab}"}, 1)) + // require.NoError(t, tree.insert("GET", "/foo/", "", 1, &Route{path: "/foo/"})) - require.NoError(t, tree.insert("GET", "/foo/*{args}", "", 1, &Route{path: "/foo/*{args}"})) + // require.NoError(t, tree.insert("GET", "/foo/*{args}", "", 1, &Route{path: "/foo/*{args}"})) // require.NoError(t, tree.insert("GET", "/{x}/a/b/c/trackk", "", 1, &Route{path: "/{x}/a/b/c/trackk"})) + // require.NoError(t, tree.insert("GET", &Route{path: "/foo/*{args}/bam"}, 1)) + nds := *tree.nodes.Load() fmt.Println(nds[0]) - c := newTestContextTree(tree) - n, tsr := tree.lookup(nds[0].children[0].Load(), "/foo/", c, false) - fmt.Println("yolo", n, tsr, c.params) + /* c := newTestContextTree(tree) + n, tsr := tree.lookup(nds[0].children[0].Load(), "/foo/a/b/c/baz", c, false) + fmt.Println("yolo", n, tsr, c.params)*/ // TODO tests // - tsr priority rollback // - wildcard suffix & wildcard infix diff --git a/iter.go b/iter.go index 50b50be..f2e5c3b 100644 --- a/iter.go +++ b/iter.go @@ -95,7 +95,7 @@ func (it Iter) Routes(methods iter.Seq[string], path string) iter.Seq2[string, * continue } - n, tsr := it.t.lookup(nds[index], path, c, true) + n, tsr := it.t.lookup(nds[index].children[0].Load(), path, c, true) if n != nil && !tsr && n.route.path == path { if !yield(method, n.route) { return @@ -128,7 +128,7 @@ func (it Iter) Reverse(methods iter.Seq[string], path string) iter.Seq2[string, continue } - n, tsr := it.t.lookup(nds[index], path, c, true) + n, tsr := it.t.lookup(nds[index].children[0].Load(), path, c, true) if n != nil && (!tsr || n.route.redirectTrailingSlash || n.route.ignoreTrailingSlash) { if !yield(method, n.route) { return diff --git a/node.go b/node.go index aa857e6..4fbaa40 100644 --- a/node.go +++ b/node.go @@ -78,10 +78,6 @@ func (n *node) isLeaf() bool { return n.route != nil } -func (n *node) isCatchAll() bool { - return n.wildcardChildIndex >= 0 -} - func (n *node) hasWildcard() bool { return len(n.params) > 0 } @@ -191,26 +187,14 @@ func (n *node) string(space int) string { sb.WriteString(" [paramIdx=") sb.WriteString(strconv.Itoa(n.paramChildIndex)) sb.WriteByte(']') - if n.hasWildcard() { - sb.WriteString(" [") - for i, param := range n.params { - if i > 0 { - sb.WriteByte(',') - } - sb.WriteString(param.key) - sb.WriteString(" (") - sb.WriteString(strconv.Itoa(param.end)) - sb.WriteString(")") - } - sb.WriteString("]") - } - } - // TODO we have to revise that - if n.isCatchAll() { - sb.WriteString(" [catchAll]") + if n.wildcardChildIndex >= 0 { + sb.WriteString(" [wildcardIdx=") + sb.WriteString(strconv.Itoa(n.wildcardChildIndex)) + sb.WriteByte(']') } + if n.isLeaf() { sb.WriteString(" [leaf=") sb.WriteString(n.route.path) @@ -220,7 +204,7 @@ func (n *node) string(space int) string { sb.WriteString(" [") for i, param := range n.params { if i > 0 { - sb.WriteByte(',') + sb.WriteString(", ") } sb.WriteString(param.key) sb.WriteString(" (") diff --git a/tree.go b/tree.go index 834f1d2..697856c 100644 --- a/tree.go +++ b/tree.go @@ -6,7 +6,6 @@ package fox import ( "fmt" - "math" "net/http" "slices" "strings" @@ -58,19 +57,14 @@ func (t *Tree) Handle(method, path string, handler HandlerFunc, opts ...PathOpti return nil, fmt.Errorf("%w: missing or invalid http method", ErrInvalidRoute) } - p, catchAllKey, n, err := parseRoute(path) + n, err := parseRoute(path) if err != nil { return nil, err } - if n < 0 || n > math.MaxUint32 { - return nil, fmt.Errorf("params count overflows (%d)", n) - } - rte := t.newRoute(path, handler, opts...) - // nolint:gosec - if err = t.insert(method, p, catchAllKey, uint32(n), rte); err != nil { + if err = t.insert(method, rte, n); err != nil { return nil, err } return rte, nil @@ -92,13 +86,13 @@ func (t *Tree) Update(method, path string, handler HandlerFunc, opts ...PathOpti return nil, fmt.Errorf("%w: missing http method", ErrInvalidRoute) } - p, catchAllKey, _, err := parseRoute(path) + _, err := parseRoute(path) if err != nil { return nil, err } rte := t.newRoute(path, handler, opts...) - if err = t.update(method, p, catchAllKey, rte); err != nil { + if err = t.update(method, rte); err != nil { return nil, err } @@ -115,12 +109,12 @@ func (t *Tree) Delete(method, path string) error { return fmt.Errorf("%w: missing http method", ErrInvalidRoute) } - p, catchAllKey, _, err := parseRoute(path) + _, err := parseRoute(path) if err != nil { return err } - if !t.remove(method, p, catchAllKey) { + if !t.remove(method, path) { return fmt.Errorf("%w: route %s %s is not registered", ErrRouteNotFound, method, path) } @@ -141,13 +135,13 @@ func (t *Tree) Has(method, path string) bool { func (t *Tree) Route(method, path string) *Route { nds := *t.nodes.Load() index := findRootNode(method, nds) - if index < 0 { + if index < 0 || len(nds[index].children) == 0 { return nil } c := t.ctx.Get().(*cTx) c.resetNil() - n, tsr := t.lookup(nds[index], path, c, true) + n, tsr := t.lookup(nds[index].children[0].Load(), path, c, true) c.Close() if n != nil && !tsr && n.route.path == path { return n.route @@ -163,13 +157,13 @@ func (t *Tree) Route(method, path string) *Route { func (t *Tree) Reverse(method, path string) (route *Route, tsr bool) { nds := *t.nodes.Load() index := findRootNode(method, nds) - if index < 0 { + if index < 0 || len(nds[index].children) == 0 { return nil, false } c := t.ctx.Get().(*cTx) c.resetNil() - n, tsr := t.lookup(nds[index], path, c, true) + n, tsr := t.lookup(nds[index].children[0].Load(), path, c, true) c.Close() if n != nil { return n.route, tsr @@ -186,8 +180,7 @@ func (t *Tree) Reverse(method, path string) (route *Route, tsr bool) { func (t *Tree) Lookup(w ResponseWriter, r *http.Request) (route *Route, cc ContextCloser, tsr bool) { nds := *t.nodes.Load() index := findRootNode(r.Method, nds) - - if index < 0 { + if index < 0 || len(nds[index].children) == 0 { return } @@ -200,7 +193,7 @@ func (t *Tree) Lookup(w ResponseWriter, r *http.Request) (route *Route, cc Conte target = r.URL.RawPath } - n, tsr := t.lookup(nds[index], target, c, false) + n, tsr := t.lookup(nds[index].children[0].Load(), target, c, false) if n != nil { c.route = n.route c.tsr = tsr @@ -219,7 +212,7 @@ func (t *Tree) Iter() Iter { // Insert is not safe for concurrent use. The path must start by '/' and it's not validated. Use // parseRoute before. -func (t *Tree) insert(method, path, _ string, paramsN uint32, route *Route) error { +func (t *Tree) insert(method string, route *Route, paramsN uint32) error { // Note that we need a consistent view of the tree during the patching so search must imperatively be locked. if !t.race.CompareAndSwap(0, 1) { panic(ErrConcurrentAccess) @@ -236,7 +229,7 @@ func (t *Tree) insert(method, path, _ string, paramsN uint32, route *Route) erro rootNode = nds[index] } - // isCatchAll := catchAllKey != "" + path := route.path result := t.search(rootNode, path) switch result.classify() { @@ -247,10 +240,6 @@ func (t *Tree) insert(method, path, _ string, paramsN uint32, route *Route) erro // └── am // Create a new node from "st" reference and update the "te" (parent) reference to "st" node. if result.matched.isLeaf() { - // TODO it's probably no needed anymore since node key contain now also the catch-all part - /* if result.matched.isCatchAll() && isCatchAll { - return newConflictErr(method, path, getRouteConflict(result.matched)) - }*/ return fmt.Errorf("%w: new route %s %s conflict with %s", ErrRouteExist, method, route.path, result.matched.route.path) } @@ -404,13 +393,15 @@ func (t *Tree) insert(method, path, _ string, paramsN uint32, route *Route) erro } // update is not safe for concurrent use. -func (t *Tree) update(method string, path, _ string, route *Route) error { +func (t *Tree) update(method string, route *Route) error { // Note that we need a consistent view of the tree during the patching so search must imperatively be locked. if !t.race.CompareAndSwap(0, 1) { panic(ErrConcurrentAccess) } defer t.race.Store(0) + path := route.path + nds := *t.nodes.Load() index := findRootNode(method, nds) if index < 0 { @@ -422,13 +413,6 @@ func (t *Tree) update(method string, path, _ string, route *Route) error { return fmt.Errorf("%w: route %s %s is not registered", ErrRouteNotFound, method, path) } - // TODO this is probably not needed any more - /* if catchAllKey != result.matched.catchAllKey { - err := newConflictErr(method, path, catchAllKey, []string{result.matched.route.path}) - err.isUpdate = true - return err - }*/ - // We are updating an existing node (could be a leaf or not). We only need to create a new node from // the matched one with the updated/added value (handler and wildcard). n := newNodeFromRef( @@ -444,7 +428,7 @@ func (t *Tree) update(method string, path, _ string, route *Route) error { } // remove is not safe for concurrent use. -func (t *Tree) remove(method, path, _ string) bool { +func (t *Tree) remove(method, path string) bool { // Note that we need a consistent view of the tree during the patching so search must imperatively be locked. if !t.race.CompareAndSwap(0, 1) { panic(ErrConcurrentAccess) @@ -458,7 +442,7 @@ func (t *Tree) remove(method, path, _ string) bool { } result := t.search(nds[index], path) - if result.classify() != exactMatch /*|| (TODO probably not needed anymore) catchAllKey != result.matched.catchAllKey */ { + if result.classify() != exactMatch { return false } @@ -658,10 +642,12 @@ Walk: newCtx := t.ctx.Get().(*cTx) startPath := charsMatched + y := path[charsMatched:] + _ = y for { idx := strings.IndexByte(path[charsMatched:], slashDelim) - // idx >= 0, we have a next segment to evaluate - if idx >= 0 { + // idx >= 0, we have a next segment with at least one char + if idx > 0 { *newCtx.params = (*newCtx.params)[:0] charsMatched += idx x := path[charsMatched:] @@ -757,12 +743,12 @@ Walk: } // Here we have a next static segment and possibly wildcard children, so we save them for late evaluation if needed. - if current.paramChildIndex >= 0 { - *c.skipNds = append(*c.skipNds, skippedNode{current, charsMatched, paramCnt, current.paramChildIndex}) - } if current.wildcardChildIndex >= 0 { *c.skipNds = append(*c.skipNds, skippedNode{current, charsMatched, paramCnt, current.wildcardChildIndex}) } + if current.paramChildIndex >= 0 { + *c.skipNds = append(*c.skipNds, skippedNode{current, charsMatched, paramCnt, current.paramChildIndex}) + } parent = current current = current.children[idx].Load() @@ -812,22 +798,11 @@ Walk: // From here we are always in a leaf if charsMatched == len(path) { if charsMatchedInNodeFound == len(current.key) { - // Exact match, note that if we match a catch-all node - /* if !lazy && current.catchAllKey != "" { - *c.params = append(*c.params, Param{Key: current.catchAllKey, Value: path[charsMatched:]}) - // Exact match, tsr is always false - return current, false - }*/ // Exact match, tsr is always false return current, false } if charsMatchedInNodeFound < len(current.key) { // Key end mid-edge - if !lazy && current.params[paramKeyCnt].catchAll { - *c.params = append(*c.params, Param{Key: current.params[paramKeyCnt].key, Value: path[charsMatched:]}) - // Exact match, tsr is always false - return current, false - } if !tsr { if strings.HasSuffix(path, "/") { // Tsr recommendation: remove the extra trailing slash (got an exact match) @@ -878,16 +853,6 @@ Walk: // Incomplete match to end of edge if charsMatched < len(path) && charsMatchedInNodeFound == len(current.key) { - /* if current.catchAllKey != "" { - if !lazy { - *c.params = append(*c.params, Param{Key: current.catchAllKey, Value: path[charsMatched:]}) - // Same as exact match, no tsr recommendation - return current, false - } - // Same as exact match, no tsr recommendation - return current, false - } - */ // Tsr recommendation: remove the extra trailing slash (got an exact match) if !tsr { remainingKeySuffix := path[charsMatched:] @@ -919,27 +884,6 @@ Walk: Backtrack: if hasSkpNds { skipped := c.skipNds.pop() - /* if skipped.n.paramChildIndex < 0 || skipped.seen { - // skipped is catch all - current = skipped.n - *c.params = (*c.params)[:skipped.paramCnt] - - if !lazy { - *c.params = append(*c.params, Param{Key: current.catchAllKey, Value: path[skipped.pathIndex:]}) - // Same as exact match, no tsr recommendation - return current, false - } - // Same as exact match, no tsr recommendation - return current, false - }*/ - - /* // Could be a catch-all node with child param - // /foo/*{any} - // /foo/{bar} - // In this case we evaluate first the child param node and fall back to the catch-all. - if skipped.n.catchAllKey != "" { - *c.skipNds = append(*c.skipNds, skippedNode{skipped.n, skipped.pathIndex, skipped.paramCnt, true}) - }*/ parent = skipped.n current = skipped.n.children[skipped.childIndex].Load() From 59473c3ae1367e953ec1a4a3b0563819e4234071 Mon Sep 17 00:00:00 2001 From: tigerwill90 Date: Mon, 21 Oct 2024 19:26:52 +0200 Subject: [PATCH 03/15] feat: wip test are passing --- tree.go | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/tree.go b/tree.go index 697856c..c45866d 100644 --- a/tree.go +++ b/tree.go @@ -531,11 +531,7 @@ const ( bracketDelim = '{' ) -func (t *Tree) lookup(rootNode *node, path string, c *cTx, lazy bool) (n *node, tsr bool) { - /* if len(rootNode.children) == 0 { - return nil, false - }*/ - +func (t *Tree) lookup(target *node, path string, c *cTx, lazy bool) (n *node, tsr bool) { var ( charsMatched int charsMatchedInNodeFound int @@ -544,8 +540,7 @@ func (t *Tree) lookup(rootNode *node, path string, c *cTx, lazy bool) (n *node, parent *node ) - // current := rootNode.children[0].Load() - current := rootNode + current := target *c.skipNds = (*c.skipNds)[:0] Walk: @@ -556,10 +551,10 @@ Walk: break } - x := string(current.key[i]) - y := string(path[charsMatched]) - _ = x - _ = y + /* x := string(current.key[i]) + y := string(path[charsMatched]) + _ = x + _ = y*/ if current.key[i] != path[charsMatched] || path[charsMatched] == bracketDelim || path[charsMatched] == '*' { if current.key[i] == '{' { @@ -642,16 +637,16 @@ Walk: newCtx := t.ctx.Get().(*cTx) startPath := charsMatched - y := path[charsMatched:] - _ = y + /* y := path[charsMatched:] + _ = y*/ for { idx := strings.IndexByte(path[charsMatched:], slashDelim) // idx >= 0, we have a next segment with at least one char if idx > 0 { *newCtx.params = (*newCtx.params)[:0] charsMatched += idx - x := path[charsMatched:] - _ = x + /* x := path[charsMatched:] + _ = x*/ newNode, newTsr := t.lookup(interNode, path[charsMatched:], newCtx, false) if newNode != nil { t.np.Put(interNode) @@ -689,8 +684,8 @@ Walk: // char matched need to be re ajusted with probabley len(path charsMatched += len(path[charsMatched:]) // and this point len(path) == charsMatched - ctrl := len(path) == charsMatched - _ = ctrl + /* ctrl := len(path) == charsMatched + _ = ctrl*/ break Walk } From 46f1200ee9a2ff983320e47b97a4e0890c86e883 Mon Sep 17 00:00:00 2001 From: tigerwill90 Date: Mon, 21 Oct 2024 21:32:38 +0200 Subject: [PATCH 04/15] feat: fix not using interNode from pool --- fox_test.go | 15 +++++++++++++++ tree.go | 22 ++++++++-------------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/fox_test.go b/fox_test.go index a4a4252..6ff4986 100644 --- a/fox_test.go +++ b/fox_test.go @@ -509,6 +509,21 @@ func BenchmarkGithubParamsAll(b *testing.B) { } } +func BenchmarkInfixCatchAll(b *testing.B) { + f := New() + f.MustHandle(http.MethodGet, "/foo/*{bar}/baz", emptyHandler) + + req := httptest.NewRequest(http.MethodGet, "/foo/a1/b22/c333/baz", nil) + w := new(mockResponseWriter) + + b.ReportAllocs() + b.ResetTimer() + + for i := 0; i < b.N; i++ { + f.ServeHTTP(w, req) + } +} + func BenchmarkLongParam(b *testing.B) { r := New() r.MustHandle(http.MethodGet, "/foo/{very_very_very_very_very_long_param}", emptyHandler) diff --git a/tree.go b/tree.go index c45866d..1cf898a 100644 --- a/tree.go +++ b/tree.go @@ -597,20 +597,14 @@ Walk: idx := current.params[paramKeyCnt].end - charsMatchedInNodeFound var interNode *node if idx >= 0 { - // -1 since on the next incrementation, if any, 'i' are going to be incremented - // i += idx - 1 - charsMatchedInNodeFound += idx - interNode = t.np.Get().(*node) interNode.params = interNode.params[:0] - interNode = &node{ - route: current.route, - key: current.key[current.params[paramKeyCnt].end:], - childKeys: current.childKeys, - children: current.children, - paramChildIndex: current.paramChildIndex, - wildcardChildIndex: current.wildcardChildIndex, - } + interNode.route = current.route + interNode.key = current.key[current.params[paramKeyCnt].end:] + interNode.childKeys = current.childKeys + interNode.children = current.children + interNode.paramChildIndex = current.paramChildIndex + interNode.wildcardChildIndex = current.wildcardChildIndex for _, ps := range current.params[paramKeyCnt+1:] { // paramKeyCnt+1 is safe since we have at least the current infix wildcard in params interNode.params = append(interNode.params, param{ key: ps.key, @@ -621,11 +615,11 @@ Walk: }) } + charsMatchedInNodeFound += idx + } else if len(current.children) > 0 { // Infix catch all interNode = current.get(0) - // -1 since on the next incrementation, if any, 'i' are going to be incremented - // i += len(current.key[charsMatchedInNodeFound:]) - 1 charsMatchedInNodeFound += len(current.key[charsMatchedInNodeFound:]) } else { // We are in an ending catch all node with no child, so it's a direct match From 690862d58865d7e4f8ff77e49766d6b13fb8a785 Mon Sep 17 00:00:00 2001 From: tigerwill90 Date: Mon, 21 Oct 2024 21:36:49 +0200 Subject: [PATCH 05/15] feat: remove unused function --- node.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/node.go b/node.go index 4fbaa40..3c7e0cb 100644 --- a/node.go +++ b/node.go @@ -238,16 +238,6 @@ type skippedNode struct { childIndex int } -func appendCatchAll(path, catchAllKey string) string { - if catchAllKey != "" { - suffix := "*{" + catchAllKey + "}" - if !strings.HasSuffix(path, suffix) { - return path + suffix - } - } - return path -} - // param represents a parsed parameter and its end position in the path. type param struct { key string From b9d6c71228caee55767139a3ffba8a3f45ebe1d3 Mon Sep 17 00:00:00 2001 From: tigerwill90 Date: Mon, 21 Oct 2024 21:48:07 +0200 Subject: [PATCH 06/15] feat: minor API improvement --- context_test.go | 4 ++-- options.go | 7 ------- route.go | 5 +---- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/context_test.go b/context_test.go index f3946c5..91f497e 100644 --- a/context_test.go +++ b/context_test.go @@ -210,11 +210,11 @@ func TestContext_Annotations(t *testing.T) { "/foo", emptyHandler, WithAnnotations(Annotation{Key: "foo", Value: "bar"}, Annotation{Key: "foo", Value: "baz"}), - WithAnnotation("john", 1), + WithAnnotations(Annotation{Key: "john", Value: 1}), ) rte := f.Tree().Route(http.MethodGet, "/foo") require.NotNil(t, rte) - assert.Equal(t, []Annotation{{"foo", "bar"}, {"foo", "baz"}, {"john", 1}}, slices.Collect(rte.Annotations())) + assert.Equal(t, []Annotation{{Key: "foo", Value: "bar"}, {Key: "foo", Value: "baz"}, {Key: "john", Value: 1}}, slices.Collect(rte.Annotations())) } func TestContext_Clone(t *testing.T) { diff --git a/options.go b/options.go index ada69f9..b66ad81 100644 --- a/options.go +++ b/options.go @@ -233,13 +233,6 @@ func WithAnnotations(annotations ...Annotation) PathOption { }) } -// WithAnnotation attaches a single key-value annotation to a route. See also [WithAnnotations] and [Annotation] for more details. -func WithAnnotation(key string, value any) PathOption { - return pathOptionFunc(func(route *Route) { - route.annots = append(route.annots, Annotation{key, value}) - }) -} - // DefaultOptions configure the router to use the Recovery middleware for the RouteHandler scope, the Logger middleware // for AllHandlers scope and enable automatic OPTIONS response. Note that DefaultOptions push the Recovery and Logger middleware // respectively to the first and second position of the middleware chains. diff --git a/route.go b/route.go index 2847340..a1c4abe 100644 --- a/route.go +++ b/route.go @@ -5,9 +5,6 @@ import ( "strings" ) -// Annotations is a collection of Annotation key-value pairs that can be attached to routes. -type Annotations []Annotation - // Annotation represents a single key-value pair that provides metadata for a route. // Annotations are typically used to store information that can be leveraged by middleware, handlers, or external // libraries to modify or customize route behavior. @@ -25,7 +22,7 @@ type Route struct { hall HandlerFunc path string mws []middleware - annots Annotations + annots []Annotation redirectTrailingSlash bool ignoreTrailingSlash bool } From 534391182400593f1ca2786aab770964a72f2934 Mon Sep 17 00:00:00 2001 From: tigerwill90 Date: Thu, 24 Oct 2024 19:11:44 +0200 Subject: [PATCH 07/15] feat: wip infix wildcard --- context.go | 11 - fox_test.go | 1098 +++++++++++++++++++++++++++++++++++++++++++++++++-- tree.go | 120 ++++-- 3 files changed, 1154 insertions(+), 75 deletions(-) diff --git a/context.go b/context.go index 0f0119e..881e296 100644 --- a/context.go +++ b/context.go @@ -402,17 +402,6 @@ func (c *cTx) CloneWith(w ResponseWriter, r *http.Request) ContextCloser { return cp } -func copyParams(src, dst *Params) { - if cap(*src) > cap(*dst) { - // Grow dst to a least cap(src) - *dst = slices.Grow(*dst, cap(*src)) - } - // cap(dst) >= cap(src) - // now constraint into len(src) & cap(src) - *dst = (*dst)[:len(*src):cap(*src)] - copy(*dst, *src) -} - // Scope returns the HandlerScope associated with the current Context. // This indicates the scope in which the handler is being executed, such as RouteHandler, NoRouteHandler, etc. func (c *cTx) Scope() HandlerScope { diff --git a/fox_test.go b/fox_test.go index 6ff4986..d40f8cd 100644 --- a/fox_test.go +++ b/fox_test.go @@ -740,6 +740,103 @@ func TestRouterWildcard(t *testing.T) { } } +func TestEmptyCatchAll(t *testing.T) { + + cases := []struct { + name string + routes []string + path string + }{ + { + name: "infix wildcard", + routes: []string{"/foo/*{args}/bar"}, + path: "/foo/bar", + }, + { + name: "infix wildcard with children", + routes: []string{"/foo/*{args}/bar", "/foo/*{args}/caz"}, + path: "/foo/bar", + }, + { + name: "infix wildcard with static edge", + routes: []string{"/foo/*{args}/bar", "/foo/baz"}, + path: "/foo/bar", + }, + { + name: "infix wildcard and suffix wildcard", + routes: []string{"/foo/*{args}/bar", "/foo/*{args}"}, + path: "/foo/", + }, + // + { + name: "infix inflight wildcard", + routes: []string{"/foo/abc*{args}/bar"}, + path: "/foo/abc/bar", + }, + { + name: "infix inflight wildcard with children", + routes: []string{"/foo/abc*{args}/bar", "/foo/abc*{args}/caz"}, + path: "/foo/abc/bar", + }, + { + name: "infix inflight wildcard with static edge", + routes: []string{"/foo/abc*{args}/bar", "/foo/abc/baz"}, + path: "/foo/abc/bar", + }, + { + name: "infix inflight wildcard and suffix wildcard", + routes: []string{"/foo/abc*{args}/bar", "/foo/abc*{args}"}, + path: "/foo/abc", + }, + { + name: "suffix wildcard wildcard with param edge", + routes: []string{"/foo/*{args}", "/foo/{param}"}, + path: "/foo/", + }, + { + name: "suffix inflight wildcard wildcard with param edge", + routes: []string{"/foo/abc*{args}", "/foo/abc{param}"}, + path: "/foo/abc", + }, + { + name: "infix wildcard wildcard with param edge", + routes: []string{"/foo/*{args}/bar", "/foo/{param}/bar"}, + path: "/foo/bar", + }, + { + name: "infix inflight wildcard wildcard with param edge", + routes: []string{"/foo/abc*{args}/bar", "/foo/abc{param}/bar"}, + path: "/foo/abc/bar", + }, + { + name: "infix wildcard wildcard with trailing slash", + routes: []string{"/foo/*{args}/"}, + path: "/foo//", + }, + { + name: "infix inflight wildcard wildcard with trailing slash", + routes: []string{"/foo/abc*{args}/"}, + path: "/foo/abc/", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + f := New() + tree := f.Tree() + for _, rte := range tc.routes { + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + } + nds := *tree.nodes.Load() + c := newTestContextTree(tree) + n, tsr := tree.lookup(nds[0].children[0].Load(), tc.path, c, false) + require.False(t, tsr) + require.Nil(t, n) + }) + } + +} + func TestRouteWithParams(t *testing.T) { tree := New().Tree() routes := [...]string{ @@ -1524,6 +1621,16 @@ func TestUpdateRoute(t *testing.T) { routes: []string{"/foo/*{bar}", "/foo", "/foo/bar"}, update: "/foo/*{bar}", }, + { + name: "replacing infix catch all node", + routes: []string{"/foo/*{bar}/baz", "/foo", "/foo/bar"}, + update: "/foo/*{bar}/baz", + }, + { + name: "replacing infix inflight catch all node", + routes: []string{"/foo/abc*{bar}/baz", "/foo", "/foo/abc{bar}"}, + update: "/foo/abc*{bar}/baz", + }, } for _, tc := range cases { @@ -3587,31 +3694,968 @@ func onlyError[T any](_ T, err error) error { return err } -func TestX(t *testing.T) { - f := New(WithIgnoreTrailingSlash(true)) - tree := f.Tree() - - require.NoError(t, tree.insert("GET", &Route{path: "/foo/"}, 0)) - require.NoError(t, tree.insert("GET", &Route{path: "/foo/*{args}"}, 1)) - require.NoError(t, tree.insert("GET", &Route{path: "/foo/*{args}/bat"}, 1)) - require.NoError(t, tree.insert("GET", &Route{path: "/foo/{ab}"}, 1)) - - // require.NoError(t, tree.insert("GET", "/foo/", "", 1, &Route{path: "/foo/"})) - // require.NoError(t, tree.insert("GET", "/foo/*{args}", "", 1, &Route{path: "/foo/*{args}"})) - // require.NoError(t, tree.insert("GET", "/{x}/a/b/c/trackk", "", 1, &Route{path: "/{x}/a/b/c/trackk"})) - - // require.NoError(t, tree.insert("GET", &Route{path: "/foo/*{args}/bam"}, 1)) - - nds := *tree.nodes.Load() - fmt.Println(nds[0]) - /* c := newTestContextTree(tree) - n, tsr := tree.lookup(nds[0].children[0].Load(), "/foo/a/b/c/baz", c, false) - fmt.Println("yolo", n, tsr, c.params)*/ - // TODO tests - // - tsr priority rollback - // - wildcard suffix & wildcard infix - // - named parameter & wildcard infix - // - static & wildcard infix - // - empty infix wildcard - // current.key[current.params[paramKeyCnt].end:] (OK) +func TestInfixWildcard(t *testing.T) { + cases := []struct { + name string + routes []string + path string + wantPath string + wantTsr bool + wantParams []Param + }{ + { + name: "simple infix wildcard", + routes: []string{"/foo/*{args}/bar"}, + path: "/foo/a/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a", + }, + }, + }, + { + name: "simple infix wildcard with route char", + routes: []string{"/foo/*{args}/bar"}, + path: "/foo/*{args}/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "*{args}", + }, + }, + }, + { + name: "simple infix wildcard with multi segment and route char", + routes: []string{"/foo/*{args}/bar"}, + path: "/foo/*{args}/b/c/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "*{args}/b/c", + }, + }, + }, + { + name: "simple infix inflight wildcard", + routes: []string{"/foo/z*{args}/bar"}, + path: "/foo/za/bar", + wantPath: "/foo/z*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a", + }, + }, + }, + { + name: "simple infix inflight wildcard with route char", + routes: []string{"/foo/z*{args}/bar"}, + path: "/foo/z*{args}/bar", + wantPath: "/foo/z*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "*{args}", + }, + }, + }, + { + name: "simple infix inflight wildcard with multi segment", + routes: []string{"/foo/z*{args}/bar"}, + path: "/foo/za/b/c/bar", + wantPath: "/foo/z*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c", + }, + }, + }, + { + name: "simple infix inflight wildcard with multi segment and route char", + routes: []string{"/foo/z*{args}/bar"}, + path: "/foo/z*{args}/b/c/bar", + wantPath: "/foo/z*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "*{args}/b/c", + }, + }, + }, + { + name: "simple infix inflight wildcard long", + routes: []string{"/foo/xyz*{args}/bar"}, + path: "/foo/xyza/bar", + wantPath: "/foo/xyz*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a", + }, + }, + }, + { + name: "simple infix inflight wildcard with multi segment long", + routes: []string{"/foo/xyz*{args}/bar"}, + path: "/foo/xyza/b/c/bar", + wantPath: "/foo/xyz*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c", + }, + }, + }, + { + name: "overlapping infix and suffix wildcard match infix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar"}, + path: "/foo/a/b/c/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c", + }, + }, + }, + { + name: "overlapping infix and suffix wildcard match suffix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar"}, + path: "/foo/a/b/c/baz", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c/baz", + }, + }, + }, + { + name: "overlapping infix and suffix wildcard match suffix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar"}, + path: "/foo/a/b/c/barito", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c/barito", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match infix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/bar"}, + path: "/foo/a/b/c/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match suffix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/bar"}, + path: "/foo/a/b/c/bili", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c/bili", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match infix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}"}, + path: "/foo/a/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match param", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/bar"}, + path: "/foo/a/bar", + wantPath: "/foo/{ps}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "a", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match suffix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}"}, + path: "/foo/a/bili", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/bili", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match param", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}"}, + path: "/foo/a", + wantPath: "/foo/{ps}", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "a", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match param with ts", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/"}, + path: "/foo/a/", + wantPath: "/foo/{ps}/", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "a", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match suffix without ts", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/"}, + path: "/foo/a", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match suffix without ts", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}"}, + path: "/foo/a/", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/", + }, + }, + }, + { + name: "overlapping infix inflight suffix wildcard and param match param", + routes: []string{"/foo/123*{args}", "/foo/123*{args}/bar", "/foo/123{ps}/bar"}, + path: "/foo/123a/bar", + wantPath: "/foo/123{ps}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "a", + }, + }, + }, + { + name: "overlapping infix inflight suffix wildcard and param match suffix", + routes: []string{"/foo/123*{args}", "/foo/123*{args}/bar", "/foo/123{ps}"}, + path: "/foo/123a/bili", + wantPath: "/foo/123*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/bili", + }, + }, + }, + { + name: "overlapping infix inflight suffix wildcard and param match param", + routes: []string{"/foo/123*{args}", "/foo/123*{args}/bar", "/foo/123{ps}"}, + path: "/foo/123a", + wantPath: "/foo/123{ps}", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "a", + }, + }, + }, + { + name: "infix segment followed by param", + routes: []string{"/foo/*{a}/{b}"}, + path: "/foo/a/b/c/d", + wantPath: "/foo/*{a}/{b}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a/b/c", + }, + { + Key: "b", + Value: "d", + }, + }, + }, + { + name: "infix segment followed by two params", + routes: []string{"/foo/*{a}/{b}/{c}"}, + path: "/foo/a/b/c/d", + wantPath: "/foo/*{a}/{b}/{c}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a/b", + }, + { + Key: "b", + Value: "c", + }, + { + Key: "c", + Value: "d", + }, + }, + }, + { + name: "infix segment followed by one param and one wildcard", + routes: []string{"/foo/*{a}/{b}/*{c}"}, + path: "/foo/a/b/c/d", + wantPath: "/foo/*{a}/{b}/*{c}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a", + }, + { + Key: "b", + Value: "b", + }, + { + Key: "c", + Value: "c/d", + }, + }, + }, + { + name: "param followed by suffix wildcard", + routes: []string{"/foo/{a}/*{b}"}, + path: "/foo/a/b/c/d", + wantPath: "/foo/{a}/*{b}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a", + }, + { + Key: "b", + Value: "b/c/d", + }, + }, + }, + { + name: "infix inflight segment followed by param", + routes: []string{"/foo/123*{a}/{b}"}, + path: "/foo/123a/b/c/d", + wantPath: "/foo/123*{a}/{b}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a/b/c", + }, + { + Key: "b", + Value: "d", + }, + }, + }, + { + name: "inflight param followed by suffix wildcard", + routes: []string{"/foo/123{a}/*{b}"}, + path: "/foo/123a/b/c/d", + wantPath: "/foo/123{a}/*{b}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a", + }, + { + Key: "b", + Value: "b/c/d", + }, + }, + }, + { + name: "multi infix segment simple", + routes: []string{"/foo/*{$1}/bar/*{$2}/baz"}, + path: "/foo/a/bar/b/c/d/baz", + wantPath: "/foo/*{$1}/bar/*{$2}/baz", + wantTsr: false, + wantParams: Params{ + { + Key: "$1", + Value: "a", + }, + { + Key: "$2", + Value: "b/c/d", + }, + }, + }, + { + name: "multi inflight segment simple", + routes: []string{"/foo/123*{$1}/bar/456*{$2}/baz"}, + path: "/foo/123a/bar/456b/c/d/baz", + wantPath: "/foo/123*{$1}/bar/456*{$2}/baz", + wantTsr: false, + wantParams: Params{ + { + Key: "$1", + Value: "a", + }, + { + Key: "$2", + Value: "b/c/d", + }, + }, + }, + { + name: "static priority", + routes: []string{"/foo/bar/baz", "/foo/{ps}/baz", "/foo/*{any}/baz"}, + path: "/foo/bar/baz", + wantPath: "/foo/bar/baz", + wantTsr: false, + }, + { + name: "param priority", + routes: []string{"/foo/bar/baz", "/foo/{ps}/baz", "/foo/*{any}/baz"}, + path: "/foo/buzz/baz", + wantPath: "/foo/{ps}/baz", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "buzz", + }, + }, + }, + { + name: "fallback catch all", + routes: []string{"/foo/bar/baz", "/foo/{ps}/baz", "/foo/*{any}/baz"}, + path: "/foo/a/b/baz", + wantPath: "/foo/*{any}/baz", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b", + }, + }, + }, + { + name: "complex overlapping route with static priority", + routes: []string{ + "/foo/bar/baz/{$1}/jo", + "/foo/*{any}/baz/{$1}/jo", + "/foo/{ps}/baz/{$1}/jo", + }, + path: "/foo/bar/baz/1/jo", + wantPath: "/foo/bar/baz/{$1}/jo", + wantTsr: false, + wantParams: Params{ + { + Key: "$1", + Value: "1", + }, + }, + }, + { + name: "complex overlapping route with param priority", + routes: []string{ + "/foo/bar/baz/{$1}/jo", + "/foo/*{any}/baz/{$1}/jo", + "/foo/{ps}/baz/{$1}/jo", + }, + path: "/foo/bam/baz/1/jo", + wantPath: "/foo/{ps}/baz/{$1}/jo", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "bam", + }, + { + Key: "$1", + Value: "1", + }, + }, + }, + { + name: "complex overlapping route with catch all fallback", + routes: []string{ + "/foo/bar/baz/{$1}/jo", + "/foo/*{any}/baz/{$1}/jo", + "/foo/{ps}/baz/{$1}/jo", + }, + path: "/foo/a/b/c/baz/1/jo", + wantPath: "/foo/*{any}/baz/{$1}/jo", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + { + Key: "$1", + Value: "1", + }, + }, + }, + { + name: "complex overlapping route with catch all fallback", + routes: []string{ + "/foo/bar/baz/{$1}/jo", + "/foo/*{any}/baz/{$1}/john", + "/foo/{ps}/baz/{$1}/johnny", + }, + path: "/foo/a/baz/1/john", + wantPath: "/foo/*{any}/baz/{$1}/john", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a", + }, + { + Key: "$1", + Value: "1", + }, + }, + }, + { + name: "overlapping static and infix", + routes: []string{ + "/foo/*{any}/baz", + "/foo/a/b/baz", + }, + path: "/foo/a/b/baz", + wantPath: "/foo/a/b/baz", + wantTsr: false, + }, + { + name: "overlapping static and infix with catch all fallback", + routes: []string{ + "/foo/*{any}/baz", + "/foo/a/b/baz", + }, + path: "/foo/a/b/c/baz", + wantPath: "/foo/*{any}/baz", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, + }, + { + name: "infix wildcard with trailing slash", + routes: []string{ + "/foo/*{any}/", + }, + path: "/foo/a/b/c/", + wantPath: "/foo/*{any}/", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, + }, + { + name: "overlapping static and infix with most specific", + routes: []string{ + "/foo/*{any}/{a}/ddd/", + "/foo/*{any}/bbb/{d}", + }, + path: "/foo/a/b/c/bbb/ddd/", + wantPath: "/foo/*{any}/{a}/ddd/", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + { + Key: "a", + Value: "bbb", + }, + }, + }, + { + name: "infix wildcard with trailing slash", + routes: []string{ + "/foo/*{any}", + "/foo/*{any}/b/", + "/foo/*{any}/c/", + }, + path: "/foo/x/y/z/", + wantPath: "/foo/*{any}", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "x/y/z/", + }, + }, + }, + { + name: "infix wildcard with trailing slash most specific", + routes: []string{ + "/foo/*{any}", + "/foo/*{any}/", + }, + path: "/foo/x/y/z/", + wantPath: "/foo/*{any}/", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "x/y/z", + }, + }, + }, + { + name: "infix wildcard with trailing slash most specific", + routes: []string{ + "/foo/*{any}", + "/foo/*{any}/", + }, + path: "/foo/x/y/z", + wantPath: "/foo/*{any}", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "x/y/z", + }, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + f := New() + tree := f.Tree() + for _, rte := range tc.routes { + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + } + nds := *tree.nodes.Load() + c := newTestContextTree(tree) + n, tsr := tree.lookup(nds[0].children[0].Load(), tc.path, c, false) + require.NotNil(t, n) + assert.Equal(t, tc.wantPath, n.route.path) + assert.Equal(t, tc.wantTsr, tsr) + c.tsr = tsr + assert.Equal(t, tc.wantParams, slices.Collect(c.Params())) + + fmt.Println(n) + }) + } + +} + +func TestInfixWildcardTsr(t *testing.T) { + cases := []struct { + name string + routes []string + path string + wantPath string + wantTsr bool + wantParams []Param + }{ + { + name: "infix wildcard with trailing slash and tsr add", + routes: []string{ + "/foo/*{any}/", + }, + path: "/foo/a/b/c", + wantPath: "/foo/*{any}/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, + }, + { + name: "infix wildcard with tsr but skipped node match", + routes: []string{ + "/foo/*{any}/", + "/{x}/a/b/c", + }, + path: "/foo/a/b/c", + wantPath: "/{x}/a/b/c", + wantTsr: false, + wantParams: Params{ + { + Key: "x", + Value: "foo", + }, + }, + }, + { + name: "infix wildcard with tsr but skipped node does not match", + routes: []string{ + "/foo/*{any}/", + "/{x}/a/b/x", + }, + path: "/foo/a/b/c", + wantPath: "/foo/*{any}/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, + }, + { + name: "infix wildcard with trailing slash and tsr add", + routes: []string{ + "/foo/*{any}/", + "/foo/*{any}/abc", + "/foo/*{any}/bcd", + }, + path: "/foo/a/b/c/abd", + wantPath: "/foo/*{any}/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c/abd", + }, + }, + }, + { + name: "infix wildcard with sub-node tsr add fallback", + routes: []string{ + "/foo/*{any}/{a}/ddd/", + "/foo/*{any}/bbb/{d}/foo", + }, + path: "/foo/a/b/c/bbb/ddd", + wantPath: "/foo/*{any}/{a}/ddd/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + { + Key: "a", + Value: "bbb", + }, + }, + }, + { + name: "infix wildcard with sub-node tsr at depth 1 but direct match", + routes: []string{ + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbb", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/*{any}/bbb", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, + }, + { + name: "infix wildcard with sub-node tsr at depth 1 and 2 but direct match", + routes: []string{ + "/foo/*{any}/b/c/bbb/", + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbb", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/*{any}/bbb", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, + }, + { + name: "infix wildcard with sub-node tsr at depth 1 and 2 but fallback first tsr", + routes: []string{ + "/foo/*{any}/b/c/bbb/", + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbx", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/*{any}/b/c/bbb/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a", + }, + }, + }, + { + name: "infix wildcard with sub-node tsr at depth 1 and 2 but fallback first tsr", + routes: []string{ + "/foo/*{any}/", + "/foo/*{any}/b/c/bbb/", + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbx", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/*{any}/b/c/bbb/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a", + }, + }, + }, + { + name: "infix wildcard with depth 0 tsr and sub-node tsr at depth 1 fallback first tsr", + routes: []string{ + "/foo/a/b/c/bbb/", + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbx", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/a/b/c/bbb/", + wantTsr: true, + }, + { + name: "infix wildcard with depth 0 tsr and sub-node tsr at depth 1 fallback first tsr", + routes: []string{ + "/foo/{first}/b/c/bbb/", + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbx", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/{first}/b/c/bbb/", + wantTsr: true, + wantParams: Params{ + { + Key: "first", + Value: "a", + }, + }, + }, + { + name: "multi infix wildcard with sub-node tsr at depth 1 but direct match", + routes: []string{ + "/foo/*{any1}/b/c/*{any2}/d/", + "/foo/*{any1}/c/*{any2}/d", + }, + path: "/foo/a/b/c/x/y/z/d", + wantPath: "/foo/*{any1}/c/*{any2}/d", + wantTsr: false, + wantParams: Params{ + { + Key: "any1", + Value: "a/b", + }, + { + Key: "any2", + Value: "x/y/z", + }, + }, + }, + { + name: "multi infix wildcard with sub-node tsr at depth 1 and fallback first", + routes: []string{ + "/foo/*{any1}/b/c/*{any2}/d/", + "/foo/*{any1}/c/*{any2}/ff", + }, + path: "/foo/a/b/c/x/y/z/d", + wantPath: "/foo/*{any1}/b/c/*{any2}/d/", + wantTsr: true, + wantParams: Params{ + { + Key: "any1", + Value: "a", + }, + { + Key: "any2", + Value: "x/y/z", + }, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + f := New() + tree := f.Tree() + for _, rte := range tc.routes { + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + } + nds := *tree.nodes.Load() + c := newTestContextTree(tree) + n, tsr := tree.lookup(nds[0].children[0].Load(), tc.path, c, false) + require.NotNil(t, n) + assert.Equal(t, tc.wantPath, n.route.path) + assert.Equal(t, tc.wantTsr, tsr) + c.tsr = tsr + assert.Equal(t, tc.wantParams, slices.Collect(c.Params())) + + fmt.Println(n) + fmt.Println("nodes:", nds[0]) + }) + } + } diff --git a/tree.go b/tree.go index 1cf898a..873eb0f 100644 --- a/tree.go +++ b/tree.go @@ -8,6 +8,7 @@ import ( "fmt" "net/http" "slices" + "strconv" "strings" "sync" "sync/atomic" @@ -531,6 +532,8 @@ const ( bracketDelim = '{' ) +var depth int + func (t *Tree) lookup(target *node, path string, c *cTx, lazy bool) (n *node, tsr bool) { var ( charsMatched int @@ -551,10 +554,10 @@ Walk: break } - /* x := string(current.key[i]) - y := string(path[charsMatched]) - _ = x - _ = y*/ + x := string(current.key[i]) + y := string(path[charsMatched]) + _ = x + _ = y if current.key[i] != path[charsMatched] || path[charsMatched] == bracketDelim || path[charsMatched] == '*' { if current.key[i] == '{' { @@ -618,7 +621,6 @@ Walk: charsMatchedInNodeFound += idx } else if len(current.children) > 0 { - // Infix catch all interNode = current.get(0) charsMatchedInNodeFound += len(current.key[charsMatchedInNodeFound:]) } else { @@ -629,57 +631,87 @@ Walk: return current, false } - newCtx := t.ctx.Get().(*cTx) + subCtx := t.ctx.Get().(*cTx) startPath := charsMatched - /* y := path[charsMatched:] - _ = y*/ + y := path[charsMatched:] + _ = y for { idx := strings.IndexByte(path[charsMatched:], slashDelim) // idx >= 0, we have a next segment with at least one char if idx > 0 { - *newCtx.params = (*newCtx.params)[:0] + *subCtx.params = (*subCtx.params)[:0] charsMatched += idx - /* x := path[charsMatched:] - _ = x*/ - newNode, newTsr := t.lookup(interNode, path[charsMatched:], newCtx, false) - if newNode != nil { - t.np.Put(interNode) - // Reminder: we have to deal with tsrParams - if tsr && newTsr { - // TODO we have tsr match, and lookup return node with tsr, caller win because most specific - // We need to test tsr behaviour extensively - t.ctx.Put(newCtx) - break - } + x := path[charsMatched:] + _ = x + a := strconv.Itoa(depth) + _ = a + depth++ + subNode, subTsr := t.lookup(interNode, path[charsMatched:], subCtx, false) + depth-- + if subNode == nil { + // Try with next segment + charsMatched++ + continue + } - if !lazy { - *c.params = append(*c.params, Param{Key: current.params[paramKeyCnt].key, Value: path[startPath:charsMatched]}) - *c.params = append(*c.params, *newCtx.params...) + // We have a tsr opportunity + if subTsr { + // Only if no previous tsr + if !tsr { + tsr = true + /* nodeCopy := &node{ + route: subNode.route, + key: subNode.key, + childKeys: subNode.childKeys, + children: subNode.children, + params: subNode.params, + paramChildIndex: subNode.paramChildIndex, + wildcardChildIndex: subNode.wildcardChildIndex, + }*/ + n = subNode + if !lazy { + *c.tsrParams = (*c.tsrParams)[:0] + *c.tsrParams = append(*c.tsrParams, *c.params...) + *c.tsrParams = append(*c.tsrParams, Param{Key: current.params[paramKeyCnt].key, Value: path[startPath:charsMatched]}) + *c.tsrParams = append(*c.tsrParams, *subCtx.tsrParams...) + } } - t.ctx.Put(newCtx) - return newNode, newTsr + + // Try with next segment + charsMatched++ + continue } - charsMatched++ - // Try with next segment - continue + + if !lazy { + *c.params = append(*c.params, Param{Key: current.params[paramKeyCnt].key, Value: path[startPath:charsMatched]}) + *c.params = append(*c.params, *subCtx.params...) + } + + t.np.Put(interNode) + t.ctx.Put(subCtx) + return subNode, subTsr } - t.ctx.Put(newCtx) - t.np.Put(interNode) + t.ctx.Put(subCtx) + // t.np.Put(interNode) + + // We can record params here because it may be either an ending catch-all node (leaf=/foo/*{args}) with + // children, or we may have a tsr opportunity (leaf=/foo/*{args}/ with /foo/x/y/z path). Note that if + // there is no tsr opportunity, and skipped nodes > 0, we will truncate the params anyway. + if !lazy { + *c.params = append(*c.params, Param{Key: current.params[paramKeyCnt].key, Value: path[startPath:]}) + paramCnt++ + } // We are also in an ending catch all, and this is the most specific path if current.params[paramKeyCnt].end == -1 { - if !lazy { - *c.params = append(*c.params, Param{Key: current.params[paramKeyCnt].key, Value: path[startPath:]}) - } return current, false } - // char matched need to be re ajusted with probabley len(path charsMatched += len(path[charsMatched:]) // and this point len(path) == charsMatched - /* ctrl := len(path) == charsMatched - _ = ctrl*/ + ctrl := len(path) == charsMatched + _ = ctrl break Walk } @@ -745,6 +777,9 @@ Walk: } } + a := strconv.Itoa(depth) + _ = a + paramCnt = 0 paramKeyCnt = 0 hasSkpNds := len(*c.skipNds) > 0 @@ -1038,3 +1073,14 @@ func (t *Tree) newRoute(path string, handler HandlerFunc, opts ...PathOption) *R return rte } + +func copyParams(src, dst *Params) { + if cap(*src) > cap(*dst) { + // Grow dst to a least cap(src) + *dst = slices.Grow(*dst, cap(*src)) + } + // cap(dst) >= cap(src) + // now constraint into len(src) & cap(src) + *dst = (*dst)[:len(*src):cap(*src)] + copy(*dst, *src) +} From ee7edb257e44ea6ced3e31fc08d59422801c6d23 Mon Sep 17 00:00:00 2001 From: tigerwill90 Date: Thu, 24 Oct 2024 21:02:43 +0200 Subject: [PATCH 08/15] feat: cleanup infix wildcard --- fox.go | 3 - fox_test.go | 5605 ++++++++++++++++++++++++++------------------------- tree.go | 58 +- 3 files changed, 2849 insertions(+), 2817 deletions(-) diff --git a/fox.go b/fox.go index d9f295e..b6af9bf 100644 --- a/fox.go +++ b/fox.go @@ -189,9 +189,6 @@ func (fox *Router) NewTree() *Tree { return tree.allocateContext() }, } - tree.np = sync.Pool{ - New: func() any { return tree.allocateNode() }, - } return tree } diff --git a/fox_test.go b/fox_test.go index d40f8cd..b387dd8 100644 --- a/fox_test.go +++ b/fox_test.go @@ -1398,3264 +1398,3331 @@ func TestOverlappingRoute(t *testing.T) { } } -func TestInsertConflict(t *testing.T) { +func TestInfixWildcard(t *testing.T) { cases := []struct { - name string - routes []struct { - wantErr error - path string - wantMatch []string - } + name string + routes []string + path string + wantPath string + wantTsr bool + wantParams []Param }{ { - name: "exact match conflict", - routes: []struct { - wantErr error - path string - wantMatch []string - }{ - {path: "/john/*{x}", wantErr: nil, wantMatch: nil}, - {path: "/john/*{y}", wantErr: ErrRouteConflict, wantMatch: []string{"/john/*{x}"}}, - {path: "/john/", wantErr: nil, wantMatch: nil}, - {path: "/foo/baz", wantErr: nil, wantMatch: nil}, - {path: "/foo/bar", wantErr: nil, wantMatch: nil}, - {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, - {path: "/foo/*{args}", wantErr: nil, wantMatch: nil}, - {path: "/avengers/ironman/{power}", wantErr: nil, wantMatch: nil}, - {path: "/avengers/{id}/bar", wantErr: nil, wantMatch: nil}, - {path: "/avengers/{id}/foo", wantErr: nil, wantMatch: nil}, - {path: "/avengers/*{args}", wantErr: nil, wantMatch: nil}, - {path: "/fox/", wantErr: nil, wantMatch: nil}, - {path: "/fox/*{args}", wantErr: nil, wantMatch: nil}, + name: "simple infix wildcard", + routes: []string{"/foo/*{args}/bar"}, + path: "/foo/a/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a", + }, }, }, { - name: "no conflict for incomplete match to end of edge", - routes: []struct { - wantErr error - path string - wantMatch []string - }{ - {path: "/foo/bar", wantErr: nil, wantMatch: nil}, - {path: "/foo/baz", wantErr: nil, wantMatch: nil}, - {path: "/foo/*{args}", wantErr: nil, wantMatch: nil}, - {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, + name: "simple infix wildcard with route char", + routes: []string{"/foo/*{args}/bar"}, + path: "/foo/*{args}/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "*{args}", + }, }, }, { - name: "no conflict for key match mid-edge", - routes: []struct { - wantErr error - path string - wantMatch []string - }{ - {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, - {path: "/foo/*{args}", wantErr: nil, wantMatch: nil}, - {path: "/foo/a*{args}", wantErr: nil, wantMatch: nil}, - {path: "/foo*{args}", wantErr: nil, wantMatch: nil}, - {path: "/john{doe}", wantErr: nil, wantMatch: nil}, - {path: "/john*{doe}", wantErr: nil, wantMatch: nil}, - {path: "/john/{doe}", wantErr: nil, wantMatch: nil}, - {path: "/joh{doe}", wantErr: nil, wantMatch: nil}, - {path: "/avengers/{id}/foo", wantErr: nil, wantMatch: nil}, - {path: "/avengers/{id}/bar", wantErr: nil, wantMatch: nil}, - {path: "/avengers/*{args}", wantErr: nil, wantMatch: nil}, + name: "simple infix wildcard with multi segment and route char", + routes: []string{"/foo/*{args}/bar"}, + path: "/foo/*{args}/b/c/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "*{args}/b/c", + }, }, }, { - name: "incomplete match to middle of edge", - routes: []struct { - wantErr error - path string - wantMatch []string - }{ - {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, - {path: "/foo/{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/foo/{id}"}}, - {path: "/gchq/*{id}", wantErr: nil, wantMatch: nil}, - {path: "/gchq/*{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/gchq/*{id}"}}, - {path: "/foo{id}", wantErr: nil, wantMatch: nil}, - {path: "/foo/a{id}", wantErr: nil, wantMatch: nil}, - {path: "/avengers/{id}/bar", wantErr: nil, wantMatch: nil}, - {path: "/avengers/{id}/baz", wantErr: nil, wantMatch: nil}, - {path: "/avengers/{id}", wantErr: nil, wantMatch: nil}, - {path: "/avengers/{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/avengers/{id}", "/avengers/{id}/bar", "/avengers/{id}/baz"}}, - {path: "/ironman/*{id}/bar", wantErr: nil, wantMatch: nil}, - {path: "/ironman/*{id}/baz", wantErr: nil, wantMatch: nil}, - {path: "/ironman/*{id}", wantErr: nil, wantMatch: nil}, - {path: "/ironman/*{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/ironman/*{id}", "/ironman/*{id}/bar", "/ironman/*{id}/baz"}}, + name: "simple infix inflight wildcard", + routes: []string{"/foo/z*{args}/bar"}, + path: "/foo/za/bar", + wantPath: "/foo/z*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a", + }, }, }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - tree := New().Tree() - for _, rte := range tc.routes { - r, err := tree.Handle(http.MethodGet, rte.path, emptyHandler) - if err != nil { - assert.Nil(t, r) - } - assert.ErrorIsf(t, err, rte.wantErr, "route: %s", rte.path) - if cErr, ok := err.(*RouteConflictError); ok { - assert.Equal(t, rte.wantMatch, cErr.Matched) - } - } - }) - } -} - -func TestUpdateConflict(t *testing.T) { - cases := []struct { - name string - routes []string - update string - wantErr error - wantMatch []string - }{ { - name: "wildcard parameter route not registered", - routes: []string{"/foo/{bar}"}, - update: "/foo/{baz}", - wantErr: ErrRouteNotFound, + name: "simple infix inflight wildcard with route char", + routes: []string{"/foo/z*{args}/bar"}, + path: "/foo/z*{args}/bar", + wantPath: "/foo/z*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "*{args}", + }, + }, }, { - name: "wildcard catch all route not registered", - routes: []string{"/foo/{bar}"}, - update: "/foo/*{baz}", - wantErr: ErrRouteNotFound, + name: "simple infix inflight wildcard with multi segment", + routes: []string{"/foo/z*{args}/bar"}, + path: "/foo/za/b/c/bar", + wantPath: "/foo/z*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c", + }, + }, }, { - name: "route match but not a leaf", - routes: []string{"/foo/bar/baz"}, - update: "/foo/bar", - wantErr: ErrRouteNotFound, + name: "simple infix inflight wildcard with multi segment and route char", + routes: []string{"/foo/z*{args}/bar"}, + path: "/foo/z*{args}/b/c/bar", + wantPath: "/foo/z*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "*{args}/b/c", + }, + }, }, { - name: "wildcard have different name", - routes: []string{"/foo/bar", "/foo/*{args}"}, - update: "/foo/*{all}", - wantErr: ErrRouteNotFound, - wantMatch: []string{"/foo/*{args}"}, + name: "simple infix inflight wildcard long", + routes: []string{"/foo/xyz*{args}/bar"}, + path: "/foo/xyza/bar", + wantPath: "/foo/xyz*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a", + }, + }, }, { - name: "replacing non wildcard by wildcard", - routes: []string{"/foo/bar", "/foo/"}, - update: "/foo/*{all}", - wantErr: ErrRouteNotFound, - wantMatch: []string{"/foo/"}, + name: "simple infix inflight wildcard with multi segment long", + routes: []string{"/foo/xyz*{args}/bar"}, + path: "/foo/xyza/b/c/bar", + wantPath: "/foo/xyz*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c", + }, + }, }, { - name: "replacing wildcard by non wildcard", - routes: []string{"/foo/bar", "/foo/*{args}"}, - update: "/foo/", - wantErr: ErrRouteNotFound, - wantMatch: []string{"/foo/*{args}"}, + name: "overlapping infix and suffix wildcard match infix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar"}, + path: "/foo/a/b/c/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c", + }, + }, }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - tree := New().Tree() - for _, rte := range tc.routes { - require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) - } - r, err := tree.Update(http.MethodGet, tc.update, emptyHandler) - if err != nil { - assert.Nil(t, r) - } - assert.ErrorIs(t, err, tc.wantErr) - }) - } -} - -func TestInvalidRoute(t *testing.T) { - f := New() - // Invalid route on insert - assert.ErrorIs(t, onlyError(f.Handle("get", "/foo", emptyHandler)), ErrInvalidRoute) - assert.ErrorIs(t, onlyError(f.Handle("", "/foo", emptyHandler)), ErrInvalidRoute) - assert.ErrorIs(t, onlyError(f.Handle(http.MethodGet, "/foo", nil)), ErrInvalidRoute) - - // Invalid route on update - assert.ErrorIs(t, onlyError(f.Update("", "/foo", emptyHandler)), ErrInvalidRoute) - assert.ErrorIs(t, onlyError(f.Update(http.MethodGet, "/foo", nil)), ErrInvalidRoute) -} - -func TestUpdateRoute(t *testing.T) { - cases := []struct { - name string - routes []string - update string - }{ { - name: "replacing ending static node", - routes: []string{"/foo/", "/foo/bar", "/foo/baz"}, - update: "/foo/bar", + name: "overlapping infix and suffix wildcard match suffix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar"}, + path: "/foo/a/b/c/baz", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c/baz", + }, + }, }, { - name: "replacing middle static node", - routes: []string{"/foo/", "/foo/bar", "/foo/baz"}, - update: "/foo/", + name: "overlapping infix and suffix wildcard match suffix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar"}, + path: "/foo/a/b/c/barito", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c/barito", + }, + }, }, { - name: "replacing ending wildcard node", - routes: []string{"/foo/", "/foo/bar", "/foo/{baz}"}, - update: "/foo/{baz}", + name: "overlapping infix suffix wildcard and param match infix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/bar"}, + path: "/foo/a/b/c/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c", + }, + }, }, { - name: "replacing ending inflight wildcard node", - routes: []string{"/foo/", "/foo/bar_xyz", "/foo/bar_{baz}"}, - update: "/foo/bar_{baz}", + name: "overlapping infix suffix wildcard and param match suffix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/bar"}, + path: "/foo/a/b/c/bili", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c/bili", + }, + }, }, { - name: "replacing middle wildcard node", - routes: []string{"/foo/{bar}", "/foo/{bar}/baz", "/foo/{bar}/xyz"}, - update: "/foo/{bar}", + name: "overlapping infix suffix wildcard and param match infix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}"}, + path: "/foo/a/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a", + }, + }, }, { - name: "replacing middle inflight wildcard node", - routes: []string{"/foo/id:{bar}", "/foo/id:{bar}/baz", "/foo/id:{bar}/xyz"}, - update: "/foo/id:{bar}", + name: "overlapping infix suffix wildcard and param match param", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/bar"}, + path: "/foo/a/bar", + wantPath: "/foo/{ps}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "a", + }, + }, }, { - name: "replacing catch all node", - routes: []string{"/foo/*{bar}", "/foo", "/foo/bar"}, - update: "/foo/*{bar}", + name: "overlapping infix suffix wildcard and param match suffix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}"}, + path: "/foo/a/bili", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/bili", + }, + }, }, { - name: "replacing infix catch all node", - routes: []string{"/foo/*{bar}/baz", "/foo", "/foo/bar"}, - update: "/foo/*{bar}/baz", + name: "overlapping infix suffix wildcard and param match param", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}"}, + path: "/foo/a", + wantPath: "/foo/{ps}", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "a", + }, + }, }, { - name: "replacing infix inflight catch all node", - routes: []string{"/foo/abc*{bar}/baz", "/foo", "/foo/abc{bar}"}, - update: "/foo/abc*{bar}/baz", + name: "overlapping infix suffix wildcard and param match param with ts", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/"}, + path: "/foo/a/", + wantPath: "/foo/{ps}/", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "a", + }, + }, }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - tree := New().Tree() - for _, rte := range tc.routes { - require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) - } - assert.NoError(t, onlyError(tree.Update(http.MethodGet, tc.update, emptyHandler))) - }) - } -} - -func TestParseRoute(t *testing.T) { - cases := []struct { - wantErr error - name string - path string - wantN uint32 - }{ { - name: "valid static route", - path: "/foo/bar", + name: "overlapping infix suffix wildcard and param match suffix without ts", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/"}, + path: "/foo/a", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a", + }, + }, }, { - name: "valid catch all route", - path: "/foo/bar/*{arg}", - wantN: 1, + name: "overlapping infix suffix wildcard and param match suffix without ts", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}"}, + path: "/foo/a/", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/", + }, + }, }, { - name: "valid param route", - path: "/foo/bar/{baz}", - wantN: 1, + name: "overlapping infix inflight suffix wildcard and param match param", + routes: []string{"/foo/123*{args}", "/foo/123*{args}/bar", "/foo/123{ps}/bar"}, + path: "/foo/123a/bar", + wantPath: "/foo/123{ps}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "a", + }, + }, }, { - name: "valid multi params route", - path: "/foo/{bar}/{baz}", - wantN: 2, + name: "overlapping infix inflight suffix wildcard and param match suffix", + routes: []string{"/foo/123*{args}", "/foo/123*{args}/bar", "/foo/123{ps}"}, + path: "/foo/123a/bili", + wantPath: "/foo/123*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/bili", + }, + }, }, { - name: "valid same params route", - path: "/foo/{bar}/{bar}", - wantN: 2, + name: "overlapping infix inflight suffix wildcard and param match param", + routes: []string{"/foo/123*{args}", "/foo/123*{args}/bar", "/foo/123{ps}"}, + path: "/foo/123a", + wantPath: "/foo/123{ps}", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "a", + }, + }, }, { - name: "valid multi params and catch all route", - path: "/foo/{bar}/{baz}/*{arg}", - wantN: 3, + name: "infix segment followed by param", + routes: []string{"/foo/*{a}/{b}"}, + path: "/foo/a/b/c/d", + wantPath: "/foo/*{a}/{b}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a/b/c", + }, + { + Key: "b", + Value: "d", + }, + }, }, { - name: "valid inflight param", - path: "/foo/xyz:{bar}", - wantN: 1, - }, - { - name: "valid inflight catchall", - path: "/foo/xyz:*{bar}", - wantN: 1, - }, - { - name: "valid multi inflight param and catch all", - path: "/foo/xyz:{bar}/abc:{bar}/*{arg}", - wantN: 3, - }, - { - name: "catch all with arg in the middle of the route", - path: "/foo/bar/*{bar}/baz", - wantN: 1, + name: "infix segment followed by two params", + routes: []string{"/foo/*{a}/{b}/{c}"}, + path: "/foo/a/b/c/d", + wantPath: "/foo/*{a}/{b}/{c}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a/b", + }, + { + Key: "b", + Value: "c", + }, + { + Key: "c", + Value: "d", + }, + }, }, { - name: "multiple catch all suffix and inflight with arg in the middle of the route", - path: "/foo/bar/*{bar}/x*{args}/y/*{z}/{b}", - wantN: 4, + name: "infix segment followed by one param and one wildcard", + routes: []string{"/foo/*{a}/{b}/*{c}"}, + path: "/foo/a/b/c/d", + wantPath: "/foo/*{a}/{b}/*{c}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a", + }, + { + Key: "b", + Value: "b", + }, + { + Key: "c", + Value: "c/d", + }, + }, }, { - name: "inflight catch all with arg in the middle of the route", - path: "/foo/bar/damn*{bar}/baz", - wantN: 1, + name: "param followed by suffix wildcard", + routes: []string{"/foo/{a}/*{b}"}, + path: "/foo/a/b/c/d", + wantPath: "/foo/{a}/*{b}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a", + }, + { + Key: "b", + Value: "b/c/d", + }, + }, }, { - name: "catch all with arg in the middle of the route and param after", - path: "/foo/bar/*{bar}/{baz}", - wantN: 2, + name: "infix inflight segment followed by param", + routes: []string{"/foo/123*{a}/{b}"}, + path: "/foo/123a/b/c/d", + wantPath: "/foo/123*{a}/{b}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a/b/c", + }, + { + Key: "b", + Value: "d", + }, + }, }, { - name: "missing prefix slash", - path: "foo/bar", - wantErr: ErrInvalidRoute, - wantN: 0, + name: "inflight param followed by suffix wildcard", + routes: []string{"/foo/123{a}/*{b}"}, + path: "/foo/123a/b/c/d", + wantPath: "/foo/123{a}/*{b}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a", + }, + { + Key: "b", + Value: "b/c/d", + }, + }, }, { - name: "empty parameter", - path: "/foo/bar{}", - wantErr: ErrInvalidRoute, - wantN: 0, + name: "multi infix segment simple", + routes: []string{"/foo/*{$1}/bar/*{$2}/baz"}, + path: "/foo/a/bar/b/c/d/baz", + wantPath: "/foo/*{$1}/bar/*{$2}/baz", + wantTsr: false, + wantParams: Params{ + { + Key: "$1", + Value: "a", + }, + { + Key: "$2", + Value: "b/c/d", + }, + }, }, { - name: "missing arguments name after catch all", - path: "/foo/bar/*", - wantErr: ErrInvalidRoute, - wantN: 0, + name: "multi inflight segment simple", + routes: []string{"/foo/123*{$1}/bar/456*{$2}/baz"}, + path: "/foo/123a/bar/456b/c/d/baz", + wantPath: "/foo/123*{$1}/bar/456*{$2}/baz", + wantTsr: false, + wantParams: Params{ + { + Key: "$1", + Value: "a", + }, + { + Key: "$2", + Value: "b/c/d", + }, + }, }, { - name: "missing arguments name after param", - path: "/foo/bar/{", - wantErr: ErrInvalidRoute, - wantN: 0, + name: "static priority", + routes: []string{"/foo/bar/baz", "/foo/{ps}/baz", "/foo/*{any}/baz"}, + path: "/foo/bar/baz", + wantPath: "/foo/bar/baz", + wantTsr: false, }, { - name: "catch all in the middle of the route", - path: "/foo/bar/*/baz", - wantErr: ErrInvalidRoute, - wantN: 0, + name: "param priority", + routes: []string{"/foo/bar/baz", "/foo/{ps}/baz", "/foo/*{any}/baz"}, + path: "/foo/buzz/baz", + wantPath: "/foo/{ps}/baz", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "buzz", + }, + }, }, { - name: "unexpected character in param", - path: "/foo/{{bar}", - wantErr: ErrInvalidRoute, - wantN: 0, + name: "fallback catch all", + routes: []string{"/foo/bar/baz", "/foo/{ps}/baz", "/foo/*{any}/baz"}, + path: "/foo/a/b/baz", + wantPath: "/foo/*{any}/baz", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b", + }, + }, }, { - name: "unexpected character in param", - path: "/foo/{*bar}", - wantErr: ErrInvalidRoute, - wantN: 0, + name: "complex overlapping route with static priority", + routes: []string{ + "/foo/bar/baz/{$1}/jo", + "/foo/*{any}/baz/{$1}/jo", + "/foo/{ps}/baz/{$1}/jo", + }, + path: "/foo/bar/baz/1/jo", + wantPath: "/foo/bar/baz/{$1}/jo", + wantTsr: false, + wantParams: Params{ + { + Key: "$1", + Value: "1", + }, + }, }, { - name: "in flight catch-all after param in one route segment", - path: "/foo/{bar}*{baz}", - wantErr: ErrInvalidRoute, - wantN: 0, + name: "complex overlapping route with param priority", + routes: []string{ + "/foo/bar/baz/{$1}/jo", + "/foo/*{any}/baz/{$1}/jo", + "/foo/{ps}/baz/{$1}/jo", + }, + path: "/foo/bam/baz/1/jo", + wantPath: "/foo/{ps}/baz/{$1}/jo", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "bam", + }, + { + Key: "$1", + Value: "1", + }, + }, }, { - name: "multiple param in one route segment", - path: "/foo/{bar}{baz}", - wantErr: ErrInvalidRoute, - wantN: 0, + name: "complex overlapping route with catch all fallback", + routes: []string{ + "/foo/bar/baz/{$1}/jo", + "/foo/*{any}/baz/{$1}/jo", + "/foo/{ps}/baz/{$1}/jo", + }, + path: "/foo/a/b/c/baz/1/jo", + wantPath: "/foo/*{any}/baz/{$1}/jo", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + { + Key: "$1", + Value: "1", + }, + }, }, { - name: "in flight param after catch all", - path: "/foo/*{args}{param}", - wantErr: ErrInvalidRoute, - wantN: 0, + name: "complex overlapping route with catch all fallback", + routes: []string{ + "/foo/bar/baz/{$1}/jo", + "/foo/*{any}/baz/{$1}/john", + "/foo/{ps}/baz/{$1}/johnny", + }, + path: "/foo/a/baz/1/john", + wantPath: "/foo/*{any}/baz/{$1}/john", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a", + }, + { + Key: "$1", + Value: "1", + }, + }, }, { - name: "consecutive catch all with no slash", - path: "/foo/*{args}*{param}", - wantErr: ErrInvalidRoute, - wantN: 0, + name: "overlapping static and infix", + routes: []string{ + "/foo/*{any}/baz", + "/foo/a/b/baz", + }, + path: "/foo/a/b/baz", + wantPath: "/foo/a/b/baz", + wantTsr: false, }, { - name: "consecutive catch all", - path: "/foo/*{args}/*{param}", - wantErr: ErrInvalidRoute, - wantN: 0, + name: "overlapping static and infix with catch all fallback", + routes: []string{ + "/foo/*{any}/baz", + "/foo/a/b/baz", + }, + path: "/foo/a/b/c/baz", + wantPath: "/foo/*{any}/baz", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, }, { - name: "consecutive catch all with inflight", - path: "/foo/ab*{args}/*{param}", - wantErr: ErrInvalidRoute, - wantN: 0, + name: "infix wildcard with trailing slash", + routes: []string{ + "/foo/*{any}/", + }, + path: "/foo/a/b/c/", + wantPath: "/foo/*{any}/", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, }, { - name: "unexpected char after inflight catch all", - path: "/foo/ab*{args}a", - wantErr: ErrInvalidRoute, - wantN: 0, + name: "overlapping static and infix with most specific", + routes: []string{ + "/foo/*{any}/{a}/ddd/", + "/foo/*{any}/bbb/{d}", + }, + path: "/foo/a/b/c/bbb/ddd/", + wantPath: "/foo/*{any}/{a}/ddd/", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + { + Key: "a", + Value: "bbb", + }, + }, }, { - name: "unexpected char after catch all", - path: "/foo/*{args}a", - wantErr: ErrInvalidRoute, - wantN: 0, + name: "infix wildcard with trailing slash", + routes: []string{ + "/foo/*{any}", + "/foo/*{any}/b/", + "/foo/*{any}/c/", + }, + path: "/foo/x/y/z/", + wantPath: "/foo/*{any}", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "x/y/z/", + }, + }, + }, + { + name: "infix wildcard with trailing slash most specific", + routes: []string{ + "/foo/*{any}", + "/foo/*{any}/", + }, + path: "/foo/x/y/z/", + wantPath: "/foo/*{any}/", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "x/y/z", + }, + }, + }, + { + name: "infix wildcard with trailing slash most specific", + routes: []string{ + "/foo/*{any}", + "/foo/*{any}/", + }, + path: "/foo/x/y/z", + wantPath: "/foo/*{any}", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "x/y/z", + }, + }, }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - n, err := parseRoute(tc.path) - require.ErrorIs(t, err, tc.wantErr) - assert.Equal(t, tc.wantN, n) + f := New() + tree := f.Tree() + for _, rte := range tc.routes { + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + } + nds := *tree.nodes.Load() + c := newTestContextTree(tree) + n, tsr := tree.lookup(nds[0].children[0].Load(), tc.path, c, false) + require.NotNil(t, n) + assert.Equal(t, tc.wantPath, n.route.path) + assert.Equal(t, tc.wantTsr, tsr) + c.tsr = tsr + assert.Equal(t, tc.wantParams, slices.Collect(c.Params())) + + fmt.Println(n) }) } + } -func TestTree_LookupTsr(t *testing.T) { +func TestInfixWildcardTsr(t *testing.T) { cases := []struct { - name string - paths []string - key string - want bool - wantPath string + name string + routes []string + path string + wantPath string + wantTsr bool + wantParams []Param }{ { - name: "match mid edge", - paths: []string{"/foo/bar/"}, - key: "/foo/bar", - want: true, - wantPath: "/foo/bar/", - }, - { - name: "incomplete match end of edge", - paths: []string{"/foo/bar"}, - key: "/foo/bar/", - want: true, - wantPath: "/foo/bar", + name: "infix wildcard with trailing slash and tsr add", + routes: []string{ + "/foo/*{any}/", + }, + path: "/foo/a/b/c", + wantPath: "/foo/*{any}/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, }, { - name: "match mid edge with child node", - paths: []string{"/users/", "/users/{id}"}, - key: "/users", - want: true, - wantPath: "/users/", + name: "infix wildcard with tsr but skipped node match", + routes: []string{ + "/foo/*{any}/", + "/{x}/a/b/c", + }, + path: "/foo/a/b/c", + wantPath: "/{x}/a/b/c", + wantTsr: false, + wantParams: Params{ + { + Key: "x", + Value: "foo", + }, + }, }, { - name: "match mid edge in child node", - paths: []string{"/users", "/users/{id}"}, - key: "/users/", - want: true, - wantPath: "/users", + name: "infix wildcard with tsr but skipped node does not match", + routes: []string{ + "/foo/*{any}/", + "/{x}/a/b/x", + }, + path: "/foo/a/b/c", + wantPath: "/foo/*{any}/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, }, { - name: "match mid edge in child node with invalid remaining prefix", - paths: []string{"/users/{id}"}, - key: "/users/", + name: "infix wildcard with trailing slash and tsr add", + routes: []string{ + "/foo/*{any}/", + "/foo/*{any}/abc", + "/foo/*{any}/bcd", + }, + path: "/foo/a/b/c/abd", + wantPath: "/foo/*{any}/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c/abd", + }, + }, }, { - name: "match mid edge with child node with invalid remaining suffix", - paths: []string{"/users/{id}"}, - key: "/users", - }, - { - name: "match mid edge with ts and more char after", - paths: []string{"/foo/bar/buzz"}, - key: "/foo/bar", - }, - { - name: "match mid edge with ts and more char before", - paths: []string{"/foo/barr/"}, - key: "/foo/bar", + name: "infix wildcard with sub-node tsr add fallback", + routes: []string{ + "/foo/*{any}/{a}/ddd/", + "/foo/*{any}/bbb/{d}/foo", + }, + path: "/foo/a/b/c/bbb/ddd", + wantPath: "/foo/*{any}/{a}/ddd/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + { + Key: "a", + Value: "bbb", + }, + }, }, { - name: "incomplete match end of edge with ts and more char after", - paths: []string{"/foo/bar"}, - key: "/foo/bar/buzz", + name: "infix wildcard with sub-node tsr at depth 1 but direct match", + routes: []string{ + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbb", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/*{any}/bbb", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, }, { - name: "incomplete match end of edge with ts and more char before", - paths: []string{"/foo/bar"}, - key: "/foo/barr/", + name: "infix wildcard with sub-node tsr at depth 1 and 2 but direct match", + routes: []string{ + "/foo/*{any}/b/c/bbb/", + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbb", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/*{any}/bbb", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - tree := New().Tree() - for _, path := range tc.paths { - require.NoError(t, tree.insert(http.MethodGet, tree.newRoute(path, emptyHandler), 0)) - } - nds := *tree.nodes.Load() - c := newTestContextTree(tree) - n, got := tree.lookup(nds[0].children[0].Load(), tc.key, c, true) - assert.Equal(t, tc.want, got) - if tc.want { - require.NotNil(t, n) - require.NotNil(t, n.route) - assert.Equal(t, tc.wantPath, n.route.path) - } - }) - } -} - -func TestRouterWithIgnoreTrailingSlash(t *testing.T) { - cases := []struct { - name string - paths []string - req string - method string - wantCode int - wantPath string - }{ { - name: "current not a leaf with extra ts", - paths: []string{"/foo", "/foo/x/", "/foo/z/"}, - req: "/foo/", - method: http.MethodGet, - wantCode: http.StatusOK, - wantPath: "/foo", + name: "infix wildcard with sub-node tsr at depth 1 and 2 but fallback first tsr", + routes: []string{ + "/foo/*{any}/b/c/bbb/", + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbx", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/*{any}/b/c/bbb/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a", + }, + }, }, { - name: "current not a leaf and path does not end with ts", - paths: []string{"/foo", "/foo/x/", "/foo/z/"}, - req: "/foo/c", - method: http.MethodGet, - wantCode: http.StatusNotFound, + name: "infix wildcard with sub-node tsr at depth 1 and 2 but fallback first tsr", + routes: []string{ + "/foo/*{any}/", + "/foo/*{any}/b/c/bbb/", + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbx", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/*{any}/b/c/bbb/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a", + }, + }, }, { - name: "current not a leaf and path end with extra char and ts", - paths: []string{"/foo", "/foo/x/", "/foo/z/"}, - req: "/foo/c/", - method: http.MethodGet, - wantCode: http.StatusNotFound, + name: "infix wildcard with depth 0 tsr and sub-node tsr at depth 1 fallback first tsr", + routes: []string{ + "/foo/a/b/c/bbb/", + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbx", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/a/b/c/bbb/", + wantTsr: true, }, { - name: "current not a leaf and path end with ts but last is not a leaf", - paths: []string{"/foo/a/a", "/foo/a/b", "/foo/c/"}, - req: "/foo/a/", - method: http.MethodGet, - wantCode: http.StatusNotFound, + name: "infix wildcard with depth 0 tsr and sub-node tsr at depth 1 fallback first tsr", + routes: []string{ + "/foo/{first}/b/c/bbb/", + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbx", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/{first}/b/c/bbb/", + wantTsr: true, + wantParams: Params{ + { + Key: "first", + Value: "a", + }, + }, }, { - name: "mid edge key with extra ts", - paths: []string{"/foo/bar/"}, - req: "/foo/bar", - method: http.MethodGet, - wantCode: http.StatusOK, - wantPath: "/foo/bar/", + name: "multi infix wildcard with sub-node tsr at depth 1 but direct match", + routes: []string{ + "/foo/*{any1}/b/c/*{any2}/d/", + "/foo/*{any1}/c/*{any2}/d", + }, + path: "/foo/a/b/c/x/y/z/d", + wantPath: "/foo/*{any1}/c/*{any2}/d", + wantTsr: false, + wantParams: Params{ + { + Key: "any1", + Value: "a/b", + }, + { + Key: "any2", + Value: "x/y/z", + }, + }, }, { - name: "mid edge key with without extra ts", - paths: []string{"/foo/bar/baz", "/foo/bar"}, - req: "/foo/bar/", - method: http.MethodGet, - wantCode: http.StatusOK, - wantPath: "/foo/bar", + name: "multi infix wildcard with sub-node tsr at depth 1 and fallback first", + routes: []string{ + "/foo/*{any1}/b/c/*{any2}/d/", + "/foo/*{any1}/c/*{any2}/x", + }, + path: "/foo/a/b/c/x/y/z/d", + wantPath: "/foo/*{any1}/b/c/*{any2}/d/", + wantTsr: true, + wantParams: Params{ + { + Key: "any1", + Value: "a", + }, + { + Key: "any2", + Value: "x/y/z", + }, + }, }, { - name: "mid edge key without extra ts", - paths: []string{"/foo/bar/baz", "/foo/bar"}, - req: "/foo/bar/", - method: http.MethodPost, - wantCode: http.StatusOK, - wantPath: "/foo/bar", + name: "multi infix wildcard with sub-node tsr and skipped nodes at depth 1 and fallback first", + routes: []string{ + "/foo/*{any1}/b/c/*{any2}/{a}/", + "/foo/*{any1}/b/c/*{any2}/d{a}/", + "/foo/*{any1}/b/c/*{any2}/dd/", + "/foo/*{any1}/c/*{any2}/x", + }, + path: "/foo/a/b/c/x/y/z/dd", + wantPath: "/foo/*{any1}/b/c/*{any2}/dd/", + wantTsr: true, + wantParams: Params{ + { + Key: "any1", + Value: "a", + }, + { + Key: "any2", + Value: "x/y/z", + }, + }, }, { - name: "incomplete match end of edge", - paths: []string{"/foo/bar"}, - req: "/foo/bar/", - method: http.MethodGet, - wantCode: http.StatusOK, - wantPath: "/foo/bar", + name: "multi infix wildcard with sub-node tsr and skipped nodes at depth 1 and direct match", + routes: []string{ + "/foo/*{any1}/b/c/*{any2}/{a}/", + "/foo/*{any1}/b/c/*{any2}/d{a}/", + "/foo/*{any1}/b/c/*{any2}/dd/", + "/foo/*{any1}/c/*{any2}/x", + }, + path: "/foo/a/b/c/x/y/z/xd/", + wantPath: "/foo/*{any1}/b/c/*{any2}/{a}/", + wantTsr: false, + wantParams: Params{ + { + Key: "any1", + Value: "a", + }, + { + Key: "any2", + Value: "x/y/z", + }, + { + Key: "a", + Value: "xd", + }, + }, }, { - name: "match mid edge with ts and more char after", - paths: []string{"/foo/bar/buzz"}, - req: "/foo/bar", - method: http.MethodGet, - wantCode: http.StatusNotFound, + name: "multi infix wildcard with sub-node tsr and skipped nodes at depth 1 with direct match depth 0", + routes: []string{ + "/foo/*{any1}/b/c/*{any2}/{a}/", + "/foo/*{any1}/b/c/*{any2}/d{a}/", + "/foo/*{any1}/b/c/*{any2}/dd/", + "/foo/*{any1}/c/*{any2}/x", + "/{a}/*{any1}/c/x/y/z/dd", + }, + path: "/foo/a/b/c/x/y/z/dd", + wantPath: "/{a}/*{any1}/c/x/y/z/dd", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "foo", + }, + { + Key: "any1", + Value: "a/b", + }, + }, }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + f := New() + tree := f.Tree() + for _, rte := range tc.routes { + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + } + nds := *tree.nodes.Load() + c := newTestContextTree(tree) + n, tsr := tree.lookup(nds[0].children[0].Load(), tc.path, c, false) + require.NotNil(t, n) + assert.Equal(t, tc.wantPath, n.route.path) + assert.Equal(t, tc.wantTsr, tsr) + c.tsr = tsr + assert.Equal(t, tc.wantParams, slices.Collect(c.Params())) + }) + } +} + +func TestInsertConflict(t *testing.T) { + cases := []struct { + name string + routes []struct { + wantErr error + path string + wantMatch []string + } + }{ { - name: "match mid edge with ts and more char before", - paths: []string{"/foo/barr/"}, - req: "/foo/bar", - method: http.MethodGet, - wantCode: http.StatusNotFound, + name: "exact match conflict", + routes: []struct { + wantErr error + path string + wantMatch []string + }{ + {path: "/john/*{x}", wantErr: nil, wantMatch: nil}, + {path: "/john/*{y}", wantErr: ErrRouteConflict, wantMatch: []string{"/john/*{x}"}}, + {path: "/john/", wantErr: nil, wantMatch: nil}, + {path: "/foo/baz", wantErr: nil, wantMatch: nil}, + {path: "/foo/bar", wantErr: nil, wantMatch: nil}, + {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, + {path: "/foo/*{args}", wantErr: nil, wantMatch: nil}, + {path: "/avengers/ironman/{power}", wantErr: nil, wantMatch: nil}, + {path: "/avengers/{id}/bar", wantErr: nil, wantMatch: nil}, + {path: "/avengers/{id}/foo", wantErr: nil, wantMatch: nil}, + {path: "/avengers/*{args}", wantErr: nil, wantMatch: nil}, + {path: "/fox/", wantErr: nil, wantMatch: nil}, + {path: "/fox/*{args}", wantErr: nil, wantMatch: nil}, + }, }, { - name: "incomplete match end of edge with ts and more char after", - paths: []string{"/foo/bar"}, - req: "/foo/bar/buzz", - method: http.MethodGet, - wantCode: http.StatusNotFound, + name: "no conflict for incomplete match to end of edge", + routes: []struct { + wantErr error + path string + wantMatch []string + }{ + {path: "/foo/bar", wantErr: nil, wantMatch: nil}, + {path: "/foo/baz", wantErr: nil, wantMatch: nil}, + {path: "/foo/*{args}", wantErr: nil, wantMatch: nil}, + {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, + }, }, { - name: "incomplete match end of edge with ts and more char before", - paths: []string{"/foo/bar"}, - req: "/foo/barr/", - method: http.MethodGet, - wantCode: http.StatusNotFound, + name: "no conflict for key match mid-edge", + routes: []struct { + wantErr error + path string + wantMatch []string + }{ + {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, + {path: "/foo/*{args}", wantErr: nil, wantMatch: nil}, + {path: "/foo/a*{args}", wantErr: nil, wantMatch: nil}, + {path: "/foo*{args}", wantErr: nil, wantMatch: nil}, + {path: "/john{doe}", wantErr: nil, wantMatch: nil}, + {path: "/john*{doe}", wantErr: nil, wantMatch: nil}, + {path: "/john/{doe}", wantErr: nil, wantMatch: nil}, + {path: "/joh{doe}", wantErr: nil, wantMatch: nil}, + {path: "/avengers/{id}/foo", wantErr: nil, wantMatch: nil}, + {path: "/avengers/{id}/bar", wantErr: nil, wantMatch: nil}, + {path: "/avengers/*{args}", wantErr: nil, wantMatch: nil}, + }, + }, + { + name: "incomplete match to middle of edge", + routes: []struct { + wantErr error + path string + wantMatch []string + }{ + {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, + {path: "/foo/{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/foo/{id}"}}, + {path: "/gchq/*{id}", wantErr: nil, wantMatch: nil}, + {path: "/gchq/*{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/gchq/*{id}"}}, + {path: "/foo{id}", wantErr: nil, wantMatch: nil}, + {path: "/foo/a{id}", wantErr: nil, wantMatch: nil}, + {path: "/avengers/{id}/bar", wantErr: nil, wantMatch: nil}, + {path: "/avengers/{id}/baz", wantErr: nil, wantMatch: nil}, + {path: "/avengers/{id}", wantErr: nil, wantMatch: nil}, + {path: "/avengers/{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/avengers/{id}", "/avengers/{id}/bar", "/avengers/{id}/baz"}}, + {path: "/ironman/*{id}/bar", wantErr: nil, wantMatch: nil}, + {path: "/ironman/*{id}/baz", wantErr: nil, wantMatch: nil}, + {path: "/ironman/*{id}", wantErr: nil, wantMatch: nil}, + {path: "/ironman/*{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/ironman/*{id}", "/ironman/*{id}/bar", "/ironman/*{id}/baz"}}, + }, }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - r := New(WithIgnoreTrailingSlash(true)) - require.True(t, r.IgnoreTrailingSlashEnabled()) - for _, path := range tc.paths { - require.NoError(t, onlyError(r.Tree().Handle(tc.method, path, func(c Context) { - _ = c.String(http.StatusOK, c.Path()) - }))) - rte := r.Tree().Route(tc.method, path) - require.NotNil(t, rte) - assert.True(t, rte.IgnoreTrailingSlashEnabled()) - } - - req := httptest.NewRequest(tc.method, tc.req, nil) - w := httptest.NewRecorder() - r.ServeHTTP(w, req) - assert.Equal(t, tc.wantCode, w.Code) - if tc.wantPath != "" { - assert.Equal(t, tc.wantPath, w.Body.String()) + tree := New().Tree() + for _, rte := range tc.routes { + r, err := tree.Handle(http.MethodGet, rte.path, emptyHandler) + if err != nil { + assert.Nil(t, r) + } + assert.ErrorIsf(t, err, rte.wantErr, "route: %s", rte.path) + if cErr, ok := err.(*RouteConflictError); ok { + assert.Equal(t, rte.wantMatch, cErr.Matched) + } } }) } } -func TestRouterWithClientIPStrategy(t *testing.T) { - c1 := ClientIPStrategyFunc(func(c Context) (*net.IPAddr, error) { - return c.RemoteIP(), nil - }) - f := New(WithClientIPStrategy(c1), WithNoRouteHandler(func(c Context) { - assert.Empty(t, c.Path()) - ip, err := c.ClientIP() - assert.NoError(t, err) - assert.NotNil(t, ip) - DefaultNotFoundHandler(c) - })) - f.MustHandle(http.MethodGet, "/foo", emptyHandler) - assert.True(t, f.ClientIPStrategyEnabled()) - - rte := f.Tree().Route(http.MethodGet, "/foo") - require.NotNil(t, rte) - assert.True(t, rte.ClientIPStrategyEnabled()) - - require.NoError(t, onlyError(f.Update(http.MethodGet, "/foo", emptyHandler, WithClientIPStrategy(nil)))) - rte = f.Tree().Route(http.MethodGet, "/foo") - require.NotNil(t, rte) - assert.False(t, rte.ClientIPStrategyEnabled()) - - // On not found handler, fallback to global ip strategy - req := httptest.NewRequest(http.MethodGet, "/bar", nil) - w := httptest.NewRecorder() - f.ServeHTTP(w, req) - assert.Equal(t, http.StatusNotFound, w.Code) -} - -func TestRedirectTrailingSlash(t *testing.T) { - +func TestUpdateConflict(t *testing.T) { cases := []struct { - name string - paths []string - req string - method string - wantCode int - wantLocation string + name string + routes []string + update string + wantErr error + wantMatch []string }{ { - name: "current not a leaf get method and status moved permanently with extra ts", - paths: []string{"/foo", "/foo/x/", "/foo/z/"}, - req: "/foo/", - method: http.MethodGet, - wantCode: http.StatusMovedPermanently, - wantLocation: "../foo", + name: "wildcard parameter route not registered", + routes: []string{"/foo/{bar}"}, + update: "/foo/{baz}", + wantErr: ErrRouteNotFound, }, { - name: "current not a leaf post method and status moved permanently with extra ts", - paths: []string{"/foo", "/foo/x/", "/foo/z/"}, - req: "/foo/", - method: http.MethodPost, - wantCode: http.StatusPermanentRedirect, - wantLocation: "../foo", + name: "wildcard catch all route not registered", + routes: []string{"/foo/{bar}"}, + update: "/foo/*{baz}", + wantErr: ErrRouteNotFound, }, { - name: "current not a leaf and path does not end with ts", - paths: []string{"/foo", "/foo/x/", "/foo/z/"}, - req: "/foo/c", - method: http.MethodGet, - wantCode: http.StatusNotFound, + name: "route match but not a leaf", + routes: []string{"/foo/bar/baz"}, + update: "/foo/bar", + wantErr: ErrRouteNotFound, }, { - name: "current not a leaf and path end with extra char and ts", - paths: []string{"/foo", "/foo/x/", "/foo/z/"}, - req: "/foo/c/", - method: http.MethodGet, - wantCode: http.StatusNotFound, + name: "wildcard have different name", + routes: []string{"/foo/bar", "/foo/*{args}"}, + update: "/foo/*{all}", + wantErr: ErrRouteNotFound, + wantMatch: []string{"/foo/*{args}"}, }, { - name: "current not a leaf and path end with ts but last is not a leaf", - paths: []string{"/foo/a/a", "/foo/a/b", "/foo/c/"}, - req: "/foo/a/", - method: http.MethodGet, - wantCode: http.StatusNotFound, + name: "replacing non wildcard by wildcard", + routes: []string{"/foo/bar", "/foo/"}, + update: "/foo/*{all}", + wantErr: ErrRouteNotFound, + wantMatch: []string{"/foo/"}, }, { - name: "mid edge key with get method and status moved permanently with extra ts", - paths: []string{"/foo/bar/"}, - req: "/foo/bar", - method: http.MethodGet, - wantCode: http.StatusMovedPermanently, - wantLocation: "bar/", + name: "replacing wildcard by non wildcard", + routes: []string{"/foo/bar", "/foo/*{args}"}, + update: "/foo/", + wantErr: ErrRouteNotFound, + wantMatch: []string{"/foo/*{args}"}, }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + tree := New().Tree() + for _, rte := range tc.routes { + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + } + r, err := tree.Update(http.MethodGet, tc.update, emptyHandler) + if err != nil { + assert.Nil(t, r) + } + assert.ErrorIs(t, err, tc.wantErr) + }) + } +} + +func TestInvalidRoute(t *testing.T) { + f := New() + // Invalid route on insert + assert.ErrorIs(t, onlyError(f.Handle("get", "/foo", emptyHandler)), ErrInvalidRoute) + assert.ErrorIs(t, onlyError(f.Handle("", "/foo", emptyHandler)), ErrInvalidRoute) + assert.ErrorIs(t, onlyError(f.Handle(http.MethodGet, "/foo", nil)), ErrInvalidRoute) + + // Invalid route on update + assert.ErrorIs(t, onlyError(f.Update("", "/foo", emptyHandler)), ErrInvalidRoute) + assert.ErrorIs(t, onlyError(f.Update(http.MethodGet, "/foo", nil)), ErrInvalidRoute) +} + +func TestUpdateRoute(t *testing.T) { + cases := []struct { + name string + routes []string + update string + }{ { - name: "mid edge key with post method and status permanent redirect with extra ts", - paths: []string{"/foo/bar/"}, - req: "/foo/bar", - method: http.MethodPost, - wantCode: http.StatusPermanentRedirect, - wantLocation: "bar/", + name: "replacing ending static node", + routes: []string{"/foo/", "/foo/bar", "/foo/baz"}, + update: "/foo/bar", }, { - name: "mid edge key with get method and status moved permanently without extra ts", - paths: []string{"/foo/bar/baz", "/foo/bar"}, - req: "/foo/bar/", - method: http.MethodGet, - wantCode: http.StatusMovedPermanently, - wantLocation: "../bar", + name: "replacing middle static node", + routes: []string{"/foo/", "/foo/bar", "/foo/baz"}, + update: "/foo/", }, { - name: "mid edge key with post method and status permanent redirect without extra ts", - paths: []string{"/foo/bar/baz", "/foo/bar"}, - req: "/foo/bar/", - method: http.MethodPost, - wantCode: http.StatusPermanentRedirect, - wantLocation: "../bar", + name: "replacing ending wildcard node", + routes: []string{"/foo/", "/foo/bar", "/foo/{baz}"}, + update: "/foo/{baz}", }, { - name: "incomplete match end of edge with get method", - paths: []string{"/foo/bar"}, - req: "/foo/bar/", - method: http.MethodGet, - wantCode: http.StatusMovedPermanently, - wantLocation: "../bar", + name: "replacing ending inflight wildcard node", + routes: []string{"/foo/", "/foo/bar_xyz", "/foo/bar_{baz}"}, + update: "/foo/bar_{baz}", }, { - name: "incomplete match end of edge with post method", - paths: []string{"/foo/bar"}, - req: "/foo/bar/", - method: http.MethodPost, - wantCode: http.StatusPermanentRedirect, - wantLocation: "../bar", + name: "replacing middle wildcard node", + routes: []string{"/foo/{bar}", "/foo/{bar}/baz", "/foo/{bar}/xyz"}, + update: "/foo/{bar}", }, { - name: "match mid edge with ts and more char after", - paths: []string{"/foo/bar/buzz"}, - req: "/foo/bar", - method: http.MethodGet, - wantCode: http.StatusNotFound, + name: "replacing middle inflight wildcard node", + routes: []string{"/foo/id:{bar}", "/foo/id:{bar}/baz", "/foo/id:{bar}/xyz"}, + update: "/foo/id:{bar}", }, { - name: "match mid edge with ts and more char before", - paths: []string{"/foo/barr/"}, - req: "/foo/bar", - method: http.MethodGet, - wantCode: http.StatusNotFound, + name: "replacing catch all node", + routes: []string{"/foo/*{bar}", "/foo", "/foo/bar"}, + update: "/foo/*{bar}", }, { - name: "incomplete match end of edge with ts and more char after", - paths: []string{"/foo/bar"}, - req: "/foo/bar/buzz", - method: http.MethodGet, - wantCode: http.StatusNotFound, + name: "replacing infix catch all node", + routes: []string{"/foo/*{bar}/baz", "/foo", "/foo/bar"}, + update: "/foo/*{bar}/baz", }, { - name: "incomplete match end of edge with ts and more char before", - paths: []string{"/foo/bar"}, - req: "/foo/barr/", - method: http.MethodGet, - wantCode: http.StatusNotFound, + name: "replacing infix inflight catch all node", + routes: []string{"/foo/abc*{bar}/baz", "/foo", "/foo/abc{bar}"}, + update: "/foo/abc*{bar}/baz", }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - r := New(WithRedirectTrailingSlash(true)) - require.True(t, r.RedirectTrailingSlashEnabled()) - for _, path := range tc.paths { - require.NoError(t, onlyError(r.Tree().Handle(tc.method, path, emptyHandler))) - rte := r.Tree().Route(tc.method, path) - require.NotNil(t, rte) - assert.True(t, rte.RedirectTrailingSlashEnabled()) - } - - req := httptest.NewRequest(tc.method, tc.req, nil) - w := httptest.NewRecorder() - r.ServeHTTP(w, req) - assert.Equal(t, tc.wantCode, w.Code) - if w.Code == http.StatusPermanentRedirect || w.Code == http.StatusMovedPermanently { - assert.Equal(t, tc.wantLocation, w.Header().Get(HeaderLocation)) - if tc.method == http.MethodGet { - assert.Equal(t, MIMETextHTMLCharsetUTF8, w.Header().Get(HeaderContentType)) - assert.Equal(t, ""+http.StatusText(w.Code)+".\n\n", w.Body.String()) - } + tree := New().Tree() + for _, rte := range tc.routes { + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) } + assert.NoError(t, onlyError(tree.Update(http.MethodGet, tc.update, emptyHandler))) }) } } -func TestEncodedRedirectTrailingSlash(t *testing.T) { - r := New(WithRedirectTrailingSlash(true)) - require.NoError(t, onlyError(r.Handle(http.MethodGet, "/foo/{bar}/", emptyHandler))) - - req := httptest.NewRequest(http.MethodGet, "/foo/bar%2Fbaz", nil) - w := httptest.NewRecorder() - - r.ServeHTTP(w, req) - assert.Equal(t, http.StatusMovedPermanently, w.Code) - assert.Equal(t, "bar%2Fbaz/", w.Header().Get(HeaderLocation)) -} - -func TestRouterWithTsrParams(t *testing.T) { +func TestParseRoute(t *testing.T) { cases := []struct { - name string - routes []string - target string - wantParams Params - wantPath string - wantTsr bool + wantErr error + name string + path string + wantN uint32 }{ { - name: "current not a leaf, with leave on incomplete to end of edge", - routes: []string{"/{a}", "/foo/{b}", "/foo/{b}/x/", "/foo/{b}/y/"}, - target: "/foo/bar/", - wantParams: Params{ - { - Key: "b", - Value: "bar", - }, - }, - wantPath: "/foo/{b}", - wantTsr: true, + name: "valid static route", + path: "/foo/bar", }, { - name: "current not a leaf, with leave on end mid-edge", - routes: []string{"/{a}/x", "/foo/{b}", "/foo/{b}/x/", "/foo/{b}/y/"}, - target: "/foo/bar/", - wantParams: Params{ - { - Key: "b", - Value: "bar", - }, - }, - wantPath: "/foo/{b}", - wantTsr: true, + name: "valid catch all route", + path: "/foo/bar/*{arg}", + wantN: 1, }, { - name: "current not a leaf, with leave on end mid-edge", - routes: []string{"/{a}/{b}/e", "/foo/{b}", "/foo/{b}/x/", "/foo/{b}/y/"}, - target: "/foo/bar/", - wantParams: Params{ - { - Key: "b", - Value: "bar", - }, - }, - wantPath: "/foo/{b}", - wantTsr: true, + name: "valid param route", + path: "/foo/bar/{baz}", + wantN: 1, }, { - name: "current not a leaf, with leave on not a leaf", - routes: []string{"/{a}/{b}/e", "/{a}/{b}/d", "/foo/{b}", "/foo/{b}/x/", "/foo/{b}/y/"}, - target: "/foo/bar/", - wantParams: Params{ - { - Key: "b", - Value: "bar", - }, - }, - wantPath: "/foo/{b}", - wantTsr: true, + name: "valid multi params route", + path: "/foo/{bar}/{baz}", + wantN: 2, }, { - name: "mid edge key, add an extra ts", - routes: []string{"/{a}", "/foo/{b}/"}, - target: "/foo/bar", - wantParams: Params{ - { - Key: "b", - Value: "bar", - }, - }, - wantPath: "/foo/{b}/", - wantTsr: true, + name: "valid same params route", + path: "/foo/{bar}/{bar}", + wantN: 2, }, { - name: "mid edge key, remove an extra ts", - routes: []string{"/{a}", "/foo/{b}/baz", "/foo/{b}"}, - target: "/foo/bar/", - wantParams: Params{ - { - Key: "b", - Value: "bar", - }, - }, - wantPath: "/foo/{b}", - wantTsr: true, + name: "valid multi params and catch all route", + path: "/foo/{bar}/{baz}/*{arg}", + wantN: 3, }, { - name: "incomplete match end of edge, remove extra ts", - routes: []string{"/{a}", "/foo/{b}"}, - target: "/foo/bar/", - wantParams: Params{ - { - Key: "b", - Value: "bar", - }, - }, - wantPath: "/foo/{b}", - wantTsr: true, + name: "valid inflight param", + path: "/foo/xyz:{bar}", + wantN: 1, }, { - name: "current not a leaf, should empty params", - routes: []string{"/{a}", "/foo", "/foo/x/", "/foo/y/"}, - target: "/foo/", - wantParams: Params(nil), - wantPath: "/foo", - wantTsr: true, + name: "valid inflight catchall", + path: "/foo/xyz:*{bar}", + wantN: 1, + }, + { + name: "valid multi inflight param and catch all", + path: "/foo/xyz:{bar}/abc:{bar}/*{arg}", + wantN: 3, + }, + { + name: "catch all with arg in the middle of the route", + path: "/foo/bar/*{bar}/baz", + wantN: 1, + }, + { + name: "multiple catch all suffix and inflight with arg in the middle of the route", + path: "/foo/bar/*{bar}/x*{args}/y/*{z}/{b}", + wantN: 4, + }, + { + name: "inflight catch all with arg in the middle of the route", + path: "/foo/bar/damn*{bar}/baz", + wantN: 1, + }, + { + name: "catch all with arg in the middle of the route and param after", + path: "/foo/bar/*{bar}/{baz}", + wantN: 2, + }, + { + name: "missing prefix slash", + path: "foo/bar", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "empty parameter", + path: "/foo/bar{}", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "missing arguments name after catch all", + path: "/foo/bar/*", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "missing arguments name after param", + path: "/foo/bar/{", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "catch all in the middle of the route", + path: "/foo/bar/*/baz", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "unexpected character in param", + path: "/foo/{{bar}", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "unexpected character in param", + path: "/foo/{*bar}", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "in flight catch-all after param in one route segment", + path: "/foo/{bar}*{baz}", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "multiple param in one route segment", + path: "/foo/{bar}{baz}", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "in flight param after catch all", + path: "/foo/*{args}{param}", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "consecutive catch all with no slash", + path: "/foo/*{args}*{param}", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "consecutive catch all", + path: "/foo/*{args}/*{param}", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "consecutive catch all with inflight", + path: "/foo/ab*{args}/*{param}", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "unexpected char after inflight catch all", + path: "/foo/ab*{args}a", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "unexpected char after catch all", + path: "/foo/*{args}a", + wantErr: ErrInvalidRoute, + wantN: 0, }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - f := New(WithIgnoreTrailingSlash(true)) - for _, rte := range tc.routes { - require.NoError(t, onlyError(f.Handle(http.MethodGet, rte, func(c Context) { - assert.Equal(t, tc.wantPath, c.Path()) - var params Params = slices.Collect(c.Params()) - assert.Equal(t, tc.wantParams, params) - assert.Equal(t, tc.wantTsr, unwrapContext(t, c).tsr) - }))) - } - req := httptest.NewRequest(http.MethodGet, tc.target, nil) - w := httptest.NewRecorder() - f.ServeHTTP(w, req) - assert.Equal(t, http.StatusOK, w.Code) + n, err := parseRoute(tc.path) + require.ErrorIs(t, err, tc.wantErr) + assert.Equal(t, tc.wantN, n) }) } - -} - -func TestTree_Delete(t *testing.T) { - tree := New().Tree() - routes := make([]route, len(githubAPI)) - copy(routes, githubAPI) - - for _, rte := range routes { - require.NoError(t, onlyError(tree.Handle(rte.method, rte.path, emptyHandler))) - } - - rand.Shuffle(len(routes), func(i, j int) { routes[i], routes[j] = routes[j], routes[i] }) - - for _, rte := range routes { - require.NoError(t, tree.Delete(rte.method, rte.path)) - } - - it := tree.Iter() - cnt := len(slices.Collect(iterutil.Right(it.All()))) - - assert.Equal(t, 0, cnt) - assert.Equal(t, 4, len(*tree.nodes.Load())) -} - -func TestTree_DeleteRoot(t *testing.T) { - tree := New().Tree() - require.NoError(t, onlyError(tree.Handle(http.MethodOptions, "/foo/bar", emptyHandler))) - require.NoError(t, tree.Delete(http.MethodOptions, "/foo/bar")) - assert.Equal(t, 4, len(*tree.nodes.Load())) -} - -func TestTree_DeleteWildcard(t *testing.T) { - f := New() - f.MustHandle(http.MethodGet, "/foo/*{args}", emptyHandler) - assert.ErrorIs(t, f.Delete(http.MethodGet, "/foo"), ErrRouteNotFound) - f.MustHandle(http.MethodGet, "/foo/{bar}", emptyHandler) - assert.NoError(t, f.Delete(http.MethodGet, "/foo/{bar}")) - assert.True(t, f.Tree().Has(http.MethodGet, "/foo/*{args}")) -} - -func TestTree_Methods(t *testing.T) { - f := New() - for _, rte := range githubAPI { - require.NoError(t, onlyError(f.Handle(rte.method, rte.path, emptyHandler))) - } - - methods := slices.Sorted(iterutil.Left(f.Iter().Reverse(f.Iter().Methods(), "/gists/123/star"))) - assert.Equal(t, []string{"DELETE", "GET", "PUT"}, methods) - - methods = slices.Sorted(f.Iter().Methods()) - assert.Equal(t, []string{"DELETE", "GET", "POST", "PUT"}, methods) - - // Ignore trailing slash disable - methods = slices.Sorted(iterutil.Left(f.Iter().Reverse(f.Iter().Methods(), "/gists/123/star/"))) - assert.Empty(t, methods) -} - -func TestTree_MethodsWithIgnoreTsEnable(t *testing.T) { - f := New(WithIgnoreTrailingSlash(true)) - for _, method := range []string{"DELETE", "GET", "PUT"} { - require.NoError(t, onlyError(f.Handle(method, "/foo/bar", emptyHandler))) - require.NoError(t, onlyError(f.Handle(method, "/john/doe/", emptyHandler))) - } - - methods := slices.Sorted(iterutil.Left(f.Iter().Reverse(f.Iter().Methods(), "/foo/bar/"))) - assert.Equal(t, []string{"DELETE", "GET", "PUT"}, methods) - - methods = slices.Sorted(iterutil.Left(f.Iter().Reverse(f.Iter().Methods(), "/john/doe"))) - assert.Equal(t, []string{"DELETE", "GET", "PUT"}, methods) - - methods = slices.Sorted(iterutil.Left(f.Iter().Reverse(f.Iter().Methods(), "/foo/bar/baz"))) - assert.Empty(t, methods) } -func TestRouterWithAllowedMethod(t *testing.T) { - r := New(WithNoMethod(true)) - +func TestTree_LookupTsr(t *testing.T) { cases := []struct { - name string - target string - path string - want string - methods []string + name string + paths []string + key string + want bool + wantPath string }{ { - name: "all route except the last one", - methods: []string{http.MethodGet, http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch, http.MethodConnect, http.MethodOptions, http.MethodHead}, - path: "/foo/bar", - target: http.MethodTrace, - want: "GET, POST, PUT, DELETE, PATCH, CONNECT, OPTIONS, HEAD", + name: "match mid edge", + paths: []string{"/foo/bar/"}, + key: "/foo/bar", + want: true, + wantPath: "/foo/bar/", }, { - name: "all route except the first one", - methods: []string{http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch, http.MethodConnect, http.MethodOptions, http.MethodHead, http.MethodTrace}, - path: "/foo/baz", - target: http.MethodGet, - want: "POST, PUT, DELETE, PATCH, CONNECT, OPTIONS, HEAD, TRACE", + name: "incomplete match end of edge", + paths: []string{"/foo/bar"}, + key: "/foo/bar/", + want: true, + wantPath: "/foo/bar", }, { - name: "all route except patch and delete", - methods: []string{http.MethodGet, http.MethodPost, http.MethodPut, http.MethodConnect, http.MethodOptions, http.MethodHead, http.MethodTrace}, - path: "/test", - target: http.MethodPatch, - want: "GET, POST, PUT, CONNECT, OPTIONS, HEAD, TRACE", + name: "match mid edge with child node", + paths: []string{"/users/", "/users/{id}"}, + key: "/users", + want: true, + wantPath: "/users/", }, - } - - require.True(t, r.MethodNotAllowedEnabled()) - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - for _, method := range tc.methods { - require.NoError(t, onlyError(r.Tree().Handle(method, tc.path, emptyHandler))) - } - req := httptest.NewRequest(tc.target, tc.path, nil) - w := httptest.NewRecorder() - r.ServeHTTP(w, req) - assert.Equal(t, http.StatusMethodNotAllowed, w.Code) - assert.Equal(t, tc.want, w.Header().Get("Allow")) - }) - } -} - -func TestRouterWithAllowedMethodAndIgnoreTsEnable(t *testing.T) { - r := New(WithNoMethod(true), WithIgnoreTrailingSlash(true)) - - // Support for ignore Trailing slash - cases := []struct { - name string - target string - path string - req string - want string - methods []string - }{ { - name: "all route except the last one", - methods: []string{http.MethodGet, http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch, http.MethodConnect, http.MethodOptions, http.MethodHead}, - path: "/foo/bar/", - req: "/foo/bar", - target: http.MethodTrace, - want: "GET, POST, PUT, DELETE, PATCH, CONNECT, OPTIONS, HEAD", + name: "match mid edge in child node", + paths: []string{"/users", "/users/{id}"}, + key: "/users/", + want: true, + wantPath: "/users", }, { - name: "all route except the first one", - methods: []string{http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch, http.MethodConnect, http.MethodOptions, http.MethodHead, http.MethodTrace}, - path: "/foo/baz", - req: "/foo/baz/", - target: http.MethodGet, - want: "POST, PUT, DELETE, PATCH, CONNECT, OPTIONS, HEAD, TRACE", + name: "match mid edge in child node with invalid remaining prefix", + paths: []string{"/users/{id}"}, + key: "/users/", }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - for _, method := range tc.methods { - require.NoError(t, onlyError(r.Tree().Handle(method, tc.path, emptyHandler))) - } - req := httptest.NewRequest(tc.target, tc.req, nil) - w := httptest.NewRecorder() - r.ServeHTTP(w, req) - assert.Equal(t, http.StatusMethodNotAllowed, w.Code) - assert.Equal(t, tc.want, w.Header().Get("Allow")) - }) - } -} - -func TestRouterWithAllowedMethodAndIgnoreTsDisable(t *testing.T) { - r := New(WithNoMethod(true)) - - // Support for ignore Trailing slash - cases := []struct { - name string - target string - path string - req string - want int - methods []string - }{ { - name: "all route except the last one", - methods: []string{http.MethodGet, http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch, http.MethodConnect, http.MethodOptions, http.MethodHead}, - path: "/foo/bar/", - req: "/foo/bar", - target: http.MethodTrace, + name: "match mid edge with child node with invalid remaining suffix", + paths: []string{"/users/{id}"}, + key: "/users", }, { - name: "all route except the first one", - methods: []string{http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch, http.MethodConnect, http.MethodOptions, http.MethodHead, http.MethodTrace}, - path: "/foo/baz", - req: "/foo/baz/", - target: http.MethodGet, + name: "match mid edge with ts and more char after", + paths: []string{"/foo/bar/buzz"}, + key: "/foo/bar", + }, + { + name: "match mid edge with ts and more char before", + paths: []string{"/foo/barr/"}, + key: "/foo/bar", + }, + { + name: "incomplete match end of edge with ts and more char after", + paths: []string{"/foo/bar"}, + key: "/foo/bar/buzz", + }, + { + name: "incomplete match end of edge with ts and more char before", + paths: []string{"/foo/bar"}, + key: "/foo/barr/", }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - for _, method := range tc.methods { - require.NoError(t, onlyError(r.Tree().Handle(method, tc.path, emptyHandler))) + tree := New().Tree() + for _, path := range tc.paths { + require.NoError(t, tree.insert(http.MethodGet, tree.newRoute(path, emptyHandler), 0)) + } + nds := *tree.nodes.Load() + c := newTestContextTree(tree) + n, got := tree.lookup(nds[0].children[0].Load(), tc.key, c, true) + assert.Equal(t, tc.want, got) + if tc.want { + require.NotNil(t, n) + require.NotNil(t, n.route) + assert.Equal(t, tc.wantPath, n.route.path) } - req := httptest.NewRequest(tc.target, tc.req, nil) - w := httptest.NewRecorder() - r.ServeHTTP(w, req) - assert.Equal(t, http.StatusNotFound, w.Code) }) } } -func TestRouterWithMethodNotAllowedHandler(t *testing.T) { - f := New(WithNoMethodHandler(func(c Context) { - c.SetHeader("FOO", "BAR") - c.Writer().WriteHeader(http.StatusMethodNotAllowed) - })) - - require.NoError(t, onlyError(f.Handle(http.MethodPost, "/foo/bar", emptyHandler))) - req := httptest.NewRequest(http.MethodGet, "/foo/bar", nil) - w := httptest.NewRecorder() - f.ServeHTTP(w, req) - assert.Equal(t, http.StatusMethodNotAllowed, w.Code) - assert.Equal(t, "POST", w.Header().Get("Allow")) - assert.Equal(t, "BAR", w.Header().Get("FOO")) -} - -func TestRouterWithAutomaticOptions(t *testing.T) { - f := New(WithAutoOptions(true)) - +func TestRouterWithIgnoreTrailingSlash(t *testing.T) { cases := []struct { name string - target string - path string - want string + paths []string + req string + method string wantCode int - methods []string + wantPath string }{ { - name: "system-wide requests", - target: "*", - path: "/foo", - methods: []string{"GET", "TRACE", "PUT"}, - want: "GET, PUT, TRACE, OPTIONS", + name: "current not a leaf with extra ts", + paths: []string{"/foo", "/foo/x/", "/foo/z/"}, + req: "/foo/", + method: http.MethodGet, wantCode: http.StatusOK, + wantPath: "/foo", }, { - name: "system-wide with custom options registered", - target: "*", - path: "/foo", - methods: []string{"GET", "TRACE", "PUT", "OPTIONS"}, - want: "GET, PUT, TRACE, OPTIONS", - wantCode: http.StatusOK, + name: "current not a leaf and path does not end with ts", + paths: []string{"/foo", "/foo/x/", "/foo/z/"}, + req: "/foo/c", + method: http.MethodGet, + wantCode: http.StatusNotFound, }, { - name: "system-wide requests with empty router", - target: "*", + name: "current not a leaf and path end with extra char and ts", + paths: []string{"/foo", "/foo/x/", "/foo/z/"}, + req: "/foo/c/", + method: http.MethodGet, wantCode: http.StatusNotFound, }, { - name: "regular option request", - target: "/foo", - path: "/foo", - methods: []string{"GET", "TRACE", "PUT"}, - want: "GET, PUT, TRACE, OPTIONS", + name: "current not a leaf and path end with ts but last is not a leaf", + paths: []string{"/foo/a/a", "/foo/a/b", "/foo/c/"}, + req: "/foo/a/", + method: http.MethodGet, + wantCode: http.StatusNotFound, + }, + { + name: "mid edge key with extra ts", + paths: []string{"/foo/bar/"}, + req: "/foo/bar", + method: http.MethodGet, wantCode: http.StatusOK, + wantPath: "/foo/bar/", }, { - name: "regular option request with handler priority", - target: "/foo", - path: "/foo", - methods: []string{"GET", "TRACE", "PUT", "OPTIONS"}, - want: "GET, OPTIONS, PUT, TRACE", - wantCode: http.StatusNoContent, + name: "mid edge key with without extra ts", + paths: []string{"/foo/bar/baz", "/foo/bar"}, + req: "/foo/bar/", + method: http.MethodGet, + wantCode: http.StatusOK, + wantPath: "/foo/bar", }, { - name: "regular option request with no matching route", - target: "/bar", - path: "/foo", - methods: []string{"GET", "TRACE", "PUT"}, + name: "mid edge key without extra ts", + paths: []string{"/foo/bar/baz", "/foo/bar"}, + req: "/foo/bar/", + method: http.MethodPost, + wantCode: http.StatusOK, + wantPath: "/foo/bar", + }, + { + name: "incomplete match end of edge", + paths: []string{"/foo/bar"}, + req: "/foo/bar/", + method: http.MethodGet, + wantCode: http.StatusOK, + wantPath: "/foo/bar", + }, + { + name: "match mid edge with ts and more char after", + paths: []string{"/foo/bar/buzz"}, + req: "/foo/bar", + method: http.MethodGet, wantCode: http.StatusNotFound, }, - } - - require.True(t, f.AutoOptionsEnabled()) - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - for _, method := range tc.methods { - require.NoError(t, onlyError(f.Tree().Handle(method, tc.path, func(c Context) { - c.SetHeader("Allow", strings.Join(slices.Sorted(iterutil.Left(c.Tree().Iter().Reverse(c.Tree().Iter().Methods(), c.Request().URL.Path))), ", ")) - c.Writer().WriteHeader(http.StatusNoContent) - }))) - } - req := httptest.NewRequest(http.MethodOptions, tc.target, nil) - w := httptest.NewRecorder() - f.ServeHTTP(w, req) + { + name: "match mid edge with ts and more char before", + paths: []string{"/foo/barr/"}, + req: "/foo/bar", + method: http.MethodGet, + wantCode: http.StatusNotFound, + }, + { + name: "incomplete match end of edge with ts and more char after", + paths: []string{"/foo/bar"}, + req: "/foo/bar/buzz", + method: http.MethodGet, + wantCode: http.StatusNotFound, + }, + { + name: "incomplete match end of edge with ts and more char before", + paths: []string{"/foo/bar"}, + req: "/foo/barr/", + method: http.MethodGet, + wantCode: http.StatusNotFound, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + r := New(WithIgnoreTrailingSlash(true)) + require.True(t, r.IgnoreTrailingSlashEnabled()) + for _, path := range tc.paths { + require.NoError(t, onlyError(r.Tree().Handle(tc.method, path, func(c Context) { + _ = c.String(http.StatusOK, c.Path()) + }))) + rte := r.Tree().Route(tc.method, path) + require.NotNil(t, rte) + assert.True(t, rte.IgnoreTrailingSlashEnabled()) + } + + req := httptest.NewRequest(tc.method, tc.req, nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) assert.Equal(t, tc.wantCode, w.Code) - assert.Equal(t, tc.want, w.Header().Get("Allow")) - // Reset - f.Swap(f.NewTree()) + if tc.wantPath != "" { + assert.Equal(t, tc.wantPath, w.Body.String()) + } }) } } -func TestRouterWithAutomaticOptionsAndIgnoreTsOptionEnable(t *testing.T) { - f := New(WithAutoOptions(true), WithIgnoreTrailingSlash(true)) +func TestRouterWithClientIPStrategy(t *testing.T) { + c1 := ClientIPStrategyFunc(func(c Context) (*net.IPAddr, error) { + return c.RemoteIP(), nil + }) + f := New(WithClientIPStrategy(c1), WithNoRouteHandler(func(c Context) { + assert.Empty(t, c.Path()) + ip, err := c.ClientIP() + assert.NoError(t, err) + assert.NotNil(t, ip) + DefaultNotFoundHandler(c) + })) + f.MustHandle(http.MethodGet, "/foo", emptyHandler) + assert.True(t, f.ClientIPStrategyEnabled()) + + rte := f.Tree().Route(http.MethodGet, "/foo") + require.NotNil(t, rte) + assert.True(t, rte.ClientIPStrategyEnabled()) + + require.NoError(t, onlyError(f.Update(http.MethodGet, "/foo", emptyHandler, WithClientIPStrategy(nil)))) + rte = f.Tree().Route(http.MethodGet, "/foo") + require.NotNil(t, rte) + assert.False(t, rte.ClientIPStrategyEnabled()) + + // On not found handler, fallback to global ip strategy + req := httptest.NewRequest(http.MethodGet, "/bar", nil) + w := httptest.NewRecorder() + f.ServeHTTP(w, req) + assert.Equal(t, http.StatusNotFound, w.Code) +} + +func TestRedirectTrailingSlash(t *testing.T) { cases := []struct { - name string - target string - path string - want string - wantCode int - methods []string + name string + paths []string + req string + method string + wantCode int + wantLocation string }{ { - name: "system-wide requests", - target: "*", - path: "/foo", - methods: []string{"GET", "TRACE", "PUT"}, - want: "GET, PUT, TRACE, OPTIONS", - wantCode: http.StatusOK, + name: "current not a leaf get method and status moved permanently with extra ts", + paths: []string{"/foo", "/foo/x/", "/foo/z/"}, + req: "/foo/", + method: http.MethodGet, + wantCode: http.StatusMovedPermanently, + wantLocation: "../foo", }, { - name: "system-wide with custom options registered", - target: "*", - path: "/foo", - methods: []string{"GET", "TRACE", "PUT", "OPTIONS"}, - want: "GET, PUT, TRACE, OPTIONS", - wantCode: http.StatusOK, + name: "current not a leaf post method and status moved permanently with extra ts", + paths: []string{"/foo", "/foo/x/", "/foo/z/"}, + req: "/foo/", + method: http.MethodPost, + wantCode: http.StatusPermanentRedirect, + wantLocation: "../foo", }, { - name: "system-wide requests with empty router", - target: "*", + name: "current not a leaf and path does not end with ts", + paths: []string{"/foo", "/foo/x/", "/foo/z/"}, + req: "/foo/c", + method: http.MethodGet, wantCode: http.StatusNotFound, }, { - name: "regular option request and ignore ts", - target: "/foo/", - path: "/foo", - methods: []string{"GET", "TRACE", "PUT"}, - want: "GET, PUT, TRACE, OPTIONS", - wantCode: http.StatusOK, + name: "current not a leaf and path end with extra char and ts", + paths: []string{"/foo", "/foo/x/", "/foo/z/"}, + req: "/foo/c/", + method: http.MethodGet, + wantCode: http.StatusNotFound, }, { - name: "regular option request with handler priority and ignore ts", - target: "/foo", - path: "/foo/", - methods: []string{"GET", "TRACE", "PUT", "OPTIONS"}, - want: "GET, OPTIONS, PUT, TRACE", - wantCode: http.StatusNoContent, + name: "current not a leaf and path end with ts but last is not a leaf", + paths: []string{"/foo/a/a", "/foo/a/b", "/foo/c/"}, + req: "/foo/a/", + method: http.MethodGet, + wantCode: http.StatusNotFound, }, { - name: "regular option request with no matching route", - target: "/bar", - path: "/foo", - methods: []string{"GET", "TRACE", "PUT"}, + name: "mid edge key with get method and status moved permanently with extra ts", + paths: []string{"/foo/bar/"}, + req: "/foo/bar", + method: http.MethodGet, + wantCode: http.StatusMovedPermanently, + wantLocation: "bar/", + }, + { + name: "mid edge key with post method and status permanent redirect with extra ts", + paths: []string{"/foo/bar/"}, + req: "/foo/bar", + method: http.MethodPost, + wantCode: http.StatusPermanentRedirect, + wantLocation: "bar/", + }, + { + name: "mid edge key with get method and status moved permanently without extra ts", + paths: []string{"/foo/bar/baz", "/foo/bar"}, + req: "/foo/bar/", + method: http.MethodGet, + wantCode: http.StatusMovedPermanently, + wantLocation: "../bar", + }, + { + name: "mid edge key with post method and status permanent redirect without extra ts", + paths: []string{"/foo/bar/baz", "/foo/bar"}, + req: "/foo/bar/", + method: http.MethodPost, + wantCode: http.StatusPermanentRedirect, + wantLocation: "../bar", + }, + { + name: "incomplete match end of edge with get method", + paths: []string{"/foo/bar"}, + req: "/foo/bar/", + method: http.MethodGet, + wantCode: http.StatusMovedPermanently, + wantLocation: "../bar", + }, + { + name: "incomplete match end of edge with post method", + paths: []string{"/foo/bar"}, + req: "/foo/bar/", + method: http.MethodPost, + wantCode: http.StatusPermanentRedirect, + wantLocation: "../bar", + }, + { + name: "match mid edge with ts and more char after", + paths: []string{"/foo/bar/buzz"}, + req: "/foo/bar", + method: http.MethodGet, + wantCode: http.StatusNotFound, + }, + { + name: "match mid edge with ts and more char before", + paths: []string{"/foo/barr/"}, + req: "/foo/bar", + method: http.MethodGet, + wantCode: http.StatusNotFound, + }, + { + name: "incomplete match end of edge with ts and more char after", + paths: []string{"/foo/bar"}, + req: "/foo/bar/buzz", + method: http.MethodGet, + wantCode: http.StatusNotFound, + }, + { + name: "incomplete match end of edge with ts and more char before", + paths: []string{"/foo/bar"}, + req: "/foo/barr/", + method: http.MethodGet, wantCode: http.StatusNotFound, }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - for _, method := range tc.methods { - require.NoError(t, onlyError(f.Tree().Handle(method, tc.path, func(c Context) { - c.SetHeader("Allow", strings.Join(slices.Sorted(iterutil.Left(c.Tree().Iter().Reverse(c.Tree().Iter().Methods(), c.Request().URL.Path))), ", ")) - c.Writer().WriteHeader(http.StatusNoContent) - }))) + r := New(WithRedirectTrailingSlash(true)) + require.True(t, r.RedirectTrailingSlashEnabled()) + for _, path := range tc.paths { + require.NoError(t, onlyError(r.Tree().Handle(tc.method, path, emptyHandler))) + rte := r.Tree().Route(tc.method, path) + require.NotNil(t, rte) + assert.True(t, rte.RedirectTrailingSlashEnabled()) } - req := httptest.NewRequest(http.MethodOptions, tc.target, nil) + + req := httptest.NewRequest(tc.method, tc.req, nil) w := httptest.NewRecorder() - f.ServeHTTP(w, req) + r.ServeHTTP(w, req) assert.Equal(t, tc.wantCode, w.Code) - assert.Equal(t, tc.want, w.Header().Get("Allow")) - // Reset - f.Swap(f.NewTree()) + if w.Code == http.StatusPermanentRedirect || w.Code == http.StatusMovedPermanently { + assert.Equal(t, tc.wantLocation, w.Header().Get(HeaderLocation)) + if tc.method == http.MethodGet { + assert.Equal(t, MIMETextHTMLCharsetUTF8, w.Header().Get(HeaderContentType)) + assert.Equal(t, ""+http.StatusText(w.Code)+".\n\n", w.Body.String()) + } + } }) } } -func TestRouterWithAutomaticOptionsAndIgnoreTsOptionDisable(t *testing.T) { - f := New(WithAutoOptions(true)) +func TestEncodedRedirectTrailingSlash(t *testing.T) { + r := New(WithRedirectTrailingSlash(true)) + require.NoError(t, onlyError(r.Handle(http.MethodGet, "/foo/{bar}/", emptyHandler))) - cases := []struct { - name string - target string - path string - wantCode int - methods []string + req := httptest.NewRequest(http.MethodGet, "/foo/bar%2Fbaz", nil) + w := httptest.NewRecorder() + + r.ServeHTTP(w, req) + assert.Equal(t, http.StatusMovedPermanently, w.Code) + assert.Equal(t, "bar%2Fbaz/", w.Header().Get(HeaderLocation)) +} + +func TestRouterWithTsrParams(t *testing.T) { + cases := []struct { + name string + routes []string + target string + wantParams Params + wantPath string + wantTsr bool }{ { - name: "regular option request and ignore ts", - target: "/foo/", - path: "/foo", - methods: []string{"GET", "TRACE", "PUT"}, - wantCode: http.StatusNotFound, + name: "current not a leaf, with leave on incomplete to end of edge", + routes: []string{"/{a}", "/foo/{b}", "/foo/{b}/x/", "/foo/{b}/y/"}, + target: "/foo/bar/", + wantParams: Params{ + { + Key: "b", + Value: "bar", + }, + }, + wantPath: "/foo/{b}", + wantTsr: true, }, { - name: "regular option request with handler priority and ignore ts", - target: "/foo", - path: "/foo/", - methods: []string{"GET", "TRACE", "PUT", "OPTIONS"}, - wantCode: http.StatusNotFound, + name: "current not a leaf, with leave on end mid-edge", + routes: []string{"/{a}/x", "/foo/{b}", "/foo/{b}/x/", "/foo/{b}/y/"}, + target: "/foo/bar/", + wantParams: Params{ + { + Key: "b", + Value: "bar", + }, + }, + wantPath: "/foo/{b}", + wantTsr: true, + }, + { + name: "current not a leaf, with leave on end mid-edge", + routes: []string{"/{a}/{b}/e", "/foo/{b}", "/foo/{b}/x/", "/foo/{b}/y/"}, + target: "/foo/bar/", + wantParams: Params{ + { + Key: "b", + Value: "bar", + }, + }, + wantPath: "/foo/{b}", + wantTsr: true, + }, + { + name: "current not a leaf, with leave on not a leaf", + routes: []string{"/{a}/{b}/e", "/{a}/{b}/d", "/foo/{b}", "/foo/{b}/x/", "/foo/{b}/y/"}, + target: "/foo/bar/", + wantParams: Params{ + { + Key: "b", + Value: "bar", + }, + }, + wantPath: "/foo/{b}", + wantTsr: true, + }, + { + name: "mid edge key, add an extra ts", + routes: []string{"/{a}", "/foo/{b}/"}, + target: "/foo/bar", + wantParams: Params{ + { + Key: "b", + Value: "bar", + }, + }, + wantPath: "/foo/{b}/", + wantTsr: true, + }, + { + name: "mid edge key, remove an extra ts", + routes: []string{"/{a}", "/foo/{b}/baz", "/foo/{b}"}, + target: "/foo/bar/", + wantParams: Params{ + { + Key: "b", + Value: "bar", + }, + }, + wantPath: "/foo/{b}", + wantTsr: true, + }, + { + name: "incomplete match end of edge, remove extra ts", + routes: []string{"/{a}", "/foo/{b}"}, + target: "/foo/bar/", + wantParams: Params{ + { + Key: "b", + Value: "bar", + }, + }, + wantPath: "/foo/{b}", + wantTsr: true, + }, + { + name: "current not a leaf, should empty params", + routes: []string{"/{a}", "/foo", "/foo/x/", "/foo/y/"}, + target: "/foo/", + wantParams: Params(nil), + wantPath: "/foo", + wantTsr: true, }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - for _, method := range tc.methods { - require.NoError(t, onlyError(f.Tree().Handle(method, tc.path, func(c Context) { - c.SetHeader("Allow", strings.Join(slices.Sorted(iterutil.Left(c.Tree().Iter().Reverse(c.Tree().Iter().Methods(), c.Request().URL.Path))), ", ")) - c.Writer().WriteHeader(http.StatusNoContent) + f := New(WithIgnoreTrailingSlash(true)) + for _, rte := range tc.routes { + require.NoError(t, onlyError(f.Handle(http.MethodGet, rte, func(c Context) { + assert.Equal(t, tc.wantPath, c.Path()) + var params Params = slices.Collect(c.Params()) + assert.Equal(t, tc.wantParams, params) + assert.Equal(t, tc.wantTsr, unwrapContext(t, c).tsr) }))) } - req := httptest.NewRequest(http.MethodOptions, tc.target, nil) + req := httptest.NewRequest(http.MethodGet, tc.target, nil) w := httptest.NewRecorder() f.ServeHTTP(w, req) - assert.Equal(t, tc.wantCode, w.Code) - // Reset - f.Swap(f.NewTree()) + assert.Equal(t, http.StatusOK, w.Code) }) } + } -func TestRouterWithOptionsHandler(t *testing.T) { - f := New(WithOptionsHandler(func(c Context) { - assert.Equal(t, "", c.Path()) - assert.Empty(t, slices.Collect(c.Params())) - c.Writer().WriteHeader(http.StatusNoContent) - })) +func TestTree_Delete(t *testing.T) { + tree := New().Tree() + routes := make([]route, len(githubAPI)) + copy(routes, githubAPI) - require.NoError(t, onlyError(f.Handle(http.MethodGet, "/foo/{bar}", emptyHandler))) - require.NoError(t, onlyError(f.Handle(http.MethodPost, "/foo/{bar}", emptyHandler))) + for _, rte := range routes { + require.NoError(t, onlyError(tree.Handle(rte.method, rte.path, emptyHandler))) + } - req := httptest.NewRequest(http.MethodOptions, "/foo/bar", nil) - w := httptest.NewRecorder() - f.ServeHTTP(w, req) - assert.Equal(t, http.StatusNoContent, w.Code) - assert.Equal(t, "GET, POST, OPTIONS", w.Header().Get("Allow")) + rand.Shuffle(len(routes), func(i, j int) { routes[i], routes[j] = routes[j], routes[i] }) + + for _, rte := range routes { + require.NoError(t, tree.Delete(rte.method, rte.path)) + } + + it := tree.Iter() + cnt := len(slices.Collect(iterutil.Right(it.All()))) + + assert.Equal(t, 0, cnt) + assert.Equal(t, 4, len(*tree.nodes.Load())) } -func TestDefaultOptions(t *testing.T) { - m := MiddlewareFunc(func(next HandlerFunc) HandlerFunc { - return func(c Context) { - next(c) - } - }) - r := New(WithMiddleware(m), DefaultOptions()) - assert.Equal(t, reflect.ValueOf(m).Pointer(), reflect.ValueOf(r.mws[2].m).Pointer()) - assert.True(t, r.handleOptions) +func TestTree_DeleteRoot(t *testing.T) { + tree := New().Tree() + require.NoError(t, onlyError(tree.Handle(http.MethodOptions, "/foo/bar", emptyHandler))) + require.NoError(t, tree.Delete(http.MethodOptions, "/foo/bar")) + assert.Equal(t, 4, len(*tree.nodes.Load())) } -func TestWithScopedMiddleware(t *testing.T) { - called := false - m := MiddlewareFunc(func(next HandlerFunc) HandlerFunc { - return func(c Context) { - called = true - next(c) - } - }) +func TestTree_DeleteWildcard(t *testing.T) { + f := New() + f.MustHandle(http.MethodGet, "/foo/*{args}", emptyHandler) + assert.ErrorIs(t, f.Delete(http.MethodGet, "/foo"), ErrRouteNotFound) + f.MustHandle(http.MethodGet, "/foo/{bar}", emptyHandler) + assert.NoError(t, f.Delete(http.MethodGet, "/foo/{bar}")) + assert.True(t, f.Tree().Has(http.MethodGet, "/foo/*{args}")) +} - r := New(WithMiddlewareFor(NoRouteHandler, m)) - require.NoError(t, onlyError(r.Handle(http.MethodGet, "/foo/bar", emptyHandler))) +func TestTree_Methods(t *testing.T) { + f := New() + for _, rte := range githubAPI { + require.NoError(t, onlyError(f.Handle(rte.method, rte.path, emptyHandler))) + } - req := httptest.NewRequest(http.MethodGet, "/foo/bar", nil) - w := httptest.NewRecorder() + methods := slices.Sorted(iterutil.Left(f.Iter().Reverse(f.Iter().Methods(), "/gists/123/star"))) + assert.Equal(t, []string{"DELETE", "GET", "PUT"}, methods) - r.ServeHTTP(w, req) - assert.False(t, called) - req.URL.Path = "/foo" - r.ServeHTTP(w, req) - assert.True(t, called) + methods = slices.Sorted(f.Iter().Methods()) + assert.Equal(t, []string{"DELETE", "GET", "POST", "PUT"}, methods) + + // Ignore trailing slash disable + methods = slices.Sorted(iterutil.Left(f.Iter().Reverse(f.Iter().Methods(), "/gists/123/star/"))) + assert.Empty(t, methods) } -func TestUpdateWithMiddleware(t *testing.T) { - called := false - m := MiddlewareFunc(func(next HandlerFunc) HandlerFunc { - return func(c Context) { - called = true - next(c) - } - }) - f := New(WithMiddleware(Recovery())) - f.MustHandle(http.MethodGet, "/foo", emptyHandler) - req := httptest.NewRequest(http.MethodGet, "/foo", nil) - w := httptest.NewRecorder() +func TestTree_MethodsWithIgnoreTsEnable(t *testing.T) { + f := New(WithIgnoreTrailingSlash(true)) + for _, method := range []string{"DELETE", "GET", "PUT"} { + require.NoError(t, onlyError(f.Handle(method, "/foo/bar", emptyHandler))) + require.NoError(t, onlyError(f.Handle(method, "/john/doe/", emptyHandler))) + } - // Add middleware - require.NoError(t, onlyError(f.Update(http.MethodGet, "/foo", emptyHandler, WithMiddleware(m)))) - f.ServeHTTP(w, req) - assert.True(t, called) - called = false + methods := slices.Sorted(iterutil.Left(f.Iter().Reverse(f.Iter().Methods(), "/foo/bar/"))) + assert.Equal(t, []string{"DELETE", "GET", "PUT"}, methods) - rte := f.Tree().Route(http.MethodGet, "/foo") - rte.Handle(newTestContextTree(f.Tree())) - assert.False(t, called) - called = false + methods = slices.Sorted(iterutil.Left(f.Iter().Reverse(f.Iter().Methods(), "/john/doe"))) + assert.Equal(t, []string{"DELETE", "GET", "PUT"}, methods) - rte.HandleMiddleware(newTestContextTree(f.Tree())) - assert.True(t, called) - called = false - - // Remove middleware - require.NoError(t, onlyError(f.Update(http.MethodGet, "/foo", emptyHandler))) - f.ServeHTTP(w, req) - assert.False(t, called) - called = false - - rte = f.Tree().Route(http.MethodGet, "/foo") - rte.Handle(newTestContextTree(f.Tree())) - assert.False(t, called) - called = false - - rte = f.Tree().Route(http.MethodGet, "/foo") - rte.HandleMiddleware(newTestContextTree(f.Tree())) - assert.False(t, called) -} - -func TestRouteMiddleware(t *testing.T) { - var c0, c1, c2 bool - m0 := MiddlewareFunc(func(next HandlerFunc) HandlerFunc { - return func(c Context) { - c0 = true - next(c) - } - }) - - m1 := MiddlewareFunc(func(next HandlerFunc) HandlerFunc { - return func(c Context) { - c1 = true - next(c) - } - }) - - m2 := MiddlewareFunc(func(next HandlerFunc) HandlerFunc { - return func(c Context) { - c2 = true - next(c) - } - }) - f := New(WithMiddleware(m0)) - f.MustHandle(http.MethodGet, "/1", emptyHandler, WithMiddleware(m1)) - f.MustHandle(http.MethodGet, "/2", emptyHandler, WithMiddleware(m2)) - - req := httptest.NewRequest(http.MethodGet, "/1", nil) - w := httptest.NewRecorder() - - f.ServeHTTP(w, req) - assert.True(t, c0) - assert.True(t, c1) - assert.False(t, c2) - c0, c1, c2 = false, false, false - - req.URL.Path = "/2" - f.ServeHTTP(w, req) - assert.True(t, c0) - assert.False(t, c1) - assert.True(t, c2) - - c0, c1, c2 = false, false, false - rte1 := f.Tree().Route(http.MethodGet, "/1") - require.NotNil(t, rte1) - rte1.Handle(newTestContextTree(f.Tree())) - assert.False(t, c0) - assert.False(t, c1) - assert.False(t, c2) - c0, c1, c2 = false, false, false - - rte1.HandleMiddleware(newTestContextTree(f.Tree())) - assert.False(t, c0) - assert.True(t, c1) - assert.False(t, c2) - c0, c1, c2 = false, false, false - - rte2 := f.Tree().Route(http.MethodGet, "/2") - require.NotNil(t, rte2) - rte2.HandleMiddleware(newTestContextTree(f.Tree())) - assert.False(t, c0) - assert.False(t, c1) - assert.True(t, c2) -} - -func TestMiddlewareLength(t *testing.T) { - f := New(DefaultOptions()) - r := f.MustHandle(http.MethodGet, "/", emptyHandler, WithMiddleware(Recovery(), Logger())) - assert.Len(t, f.mws, 2) - assert.Len(t, r.mws, 4) -} - -func TestWithNotFoundHandler(t *testing.T) { - notFound := func(c Context) { - _ = c.String(http.StatusNotFound, "NOT FOUND\n") - } - - f := New(WithNoRouteHandler(notFound)) - require.NoError(t, onlyError(f.Handle(http.MethodGet, "/foo", emptyHandler))) - - req := httptest.NewRequest(http.MethodGet, "/foo/bar", nil) - w := httptest.NewRecorder() - - f.ServeHTTP(w, req) - assert.Equal(t, http.StatusNotFound, w.Code) - assert.Equal(t, "NOT FOUND\n", w.Body.String()) -} - -func TestRouter_Lookup(t *testing.T) { - rx := regexp.MustCompile("({|\\*{)[A-z]+[}]") - f := New() - for _, rte := range githubAPI { - require.NoError(t, onlyError(f.Handle(rte.method, rte.path, emptyHandler))) - } - - for _, rte := range githubAPI { - req := httptest.NewRequest(rte.method, rte.path, nil) - route, cc, _ := f.Lookup(newResponseWriter(mockResponseWriter{}), req) - require.NotNil(t, cc) - require.NotNil(t, route) - assert.Equal(t, rte.path, route.Path()) - - matches := rx.FindAllString(rte.path, -1) - for _, match := range matches { - var key string - if strings.HasPrefix(match, "*") { - key = match[2 : len(match)-1] - } else { - key = match[1 : len(match)-1] - } - value := match - assert.Equal(t, value, cc.Param(key)) - } - - cc.Close() - } - - // No method match - req := httptest.NewRequest("ANY", "/bar", nil) - route, cc, _ := f.Lookup(newResponseWriter(mockResponseWriter{}), req) - assert.Nil(t, route) - assert.Nil(t, cc) - - // No path match - req = httptest.NewRequest(http.MethodGet, "/bar", nil) - route, cc, _ = f.Lookup(newResponseWriter(mockResponseWriter{}), req) - assert.Nil(t, route) - assert.Nil(t, cc) + methods = slices.Sorted(iterutil.Left(f.Iter().Reverse(f.Iter().Methods(), "/foo/bar/baz"))) + assert.Empty(t, methods) } -func TestTree_Has(t *testing.T) { - routes := []string{ - "/foo/bar", - "/welcome/{name}", - "/users/uid_{id}", - "/john/doe/", - } - - r := New() - for _, rte := range routes { - require.NoError(t, onlyError(r.Handle(http.MethodGet, rte, emptyHandler))) - } +func TestRouterWithAllowedMethod(t *testing.T) { + r := New(WithNoMethod(true)) cases := []struct { - name string - path string - want bool + name string + target string + path string + want string + methods []string }{ { - name: "strict match static route", - path: "/foo/bar", - want: true, - }, - { - name: "strict match static route", - path: "/john/doe/", - want: true, - }, - { - name: "no match static route (tsr)", - path: "/foo/bar/", - }, - { - name: "no match static route (tsr)", - path: "/john/doe", - }, - { - name: "strict match route params", - path: "/welcome/{name}", - want: true, - }, - { - name: "no match route params", - path: "/welcome/fox", + name: "all route except the last one", + methods: []string{http.MethodGet, http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch, http.MethodConnect, http.MethodOptions, http.MethodHead}, + path: "/foo/bar", + target: http.MethodTrace, + want: "GET, POST, PUT, DELETE, PATCH, CONNECT, OPTIONS, HEAD", }, { - name: "strict match mid route params", - path: "/users/uid_{id}", - want: true, + name: "all route except the first one", + methods: []string{http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch, http.MethodConnect, http.MethodOptions, http.MethodHead, http.MethodTrace}, + path: "/foo/baz", + target: http.MethodGet, + want: "POST, PUT, DELETE, PATCH, CONNECT, OPTIONS, HEAD, TRACE", }, { - name: "no match mid route params", - path: "/users/uid_123", + name: "all route except patch and delete", + methods: []string{http.MethodGet, http.MethodPost, http.MethodPut, http.MethodConnect, http.MethodOptions, http.MethodHead, http.MethodTrace}, + path: "/test", + target: http.MethodPatch, + want: "GET, POST, PUT, CONNECT, OPTIONS, HEAD, TRACE", }, } - tree := r.Tree() + require.True(t, r.MethodNotAllowedEnabled()) for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.want, tree.Has(http.MethodGet, tc.path)) + for _, method := range tc.methods { + require.NoError(t, onlyError(r.Tree().Handle(method, tc.path, emptyHandler))) + } + req := httptest.NewRequest(tc.target, tc.path, nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + assert.Equal(t, http.StatusMethodNotAllowed, w.Code) + assert.Equal(t, tc.want, w.Header().Get("Allow")) }) } } -func TestTree_Route(t *testing.T) { - routes := []string{ - "/foo/bar", - "/welcome/{name}", - "/users/uid_{id}", - } - - r := New() - for _, rte := range routes { - require.NoError(t, onlyError(r.Handle(http.MethodGet, rte, emptyHandler))) - } +func TestRouterWithAllowedMethodAndIgnoreTsEnable(t *testing.T) { + r := New(WithNoMethod(true), WithIgnoreTrailingSlash(true)) + // Support for ignore Trailing slash cases := []struct { name string + target string path string + req string want string - wantTsr bool + methods []string }{ { - name: "reverse static route", - path: "/foo/bar", - want: "/foo/bar", + name: "all route except the last one", + methods: []string{http.MethodGet, http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch, http.MethodConnect, http.MethodOptions, http.MethodHead}, + path: "/foo/bar/", + req: "/foo/bar", + target: http.MethodTrace, + want: "GET, POST, PUT, DELETE, PATCH, CONNECT, OPTIONS, HEAD", }, { - name: "reverse static route with tsr disable", - path: "/foo/bar/", - want: "/foo/bar", - wantTsr: true, - }, - { - name: "reverse params route", - path: "/welcome/fox", - want: "/welcome/{name}", + name: "all route except the first one", + methods: []string{http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch, http.MethodConnect, http.MethodOptions, http.MethodHead, http.MethodTrace}, + path: "/foo/baz", + req: "/foo/baz/", + target: http.MethodGet, + want: "POST, PUT, DELETE, PATCH, CONNECT, OPTIONS, HEAD, TRACE", }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + for _, method := range tc.methods { + require.NoError(t, onlyError(r.Tree().Handle(method, tc.path, emptyHandler))) + } + req := httptest.NewRequest(tc.target, tc.req, nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + assert.Equal(t, http.StatusMethodNotAllowed, w.Code) + assert.Equal(t, tc.want, w.Header().Get("Allow")) + }) + } +} + +func TestRouterWithAllowedMethodAndIgnoreTsDisable(t *testing.T) { + r := New(WithNoMethod(true)) + + // Support for ignore Trailing slash + cases := []struct { + name string + target string + path string + req string + want int + methods []string + }{ { - name: "reverse mid params route", - path: "/users/uid_123", - want: "/users/uid_{id}", + name: "all route except the last one", + methods: []string{http.MethodGet, http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch, http.MethodConnect, http.MethodOptions, http.MethodHead}, + path: "/foo/bar/", + req: "/foo/bar", + target: http.MethodTrace, }, { - name: "reverse no match", - path: "/users/fox", + name: "all route except the first one", + methods: []string{http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch, http.MethodConnect, http.MethodOptions, http.MethodHead, http.MethodTrace}, + path: "/foo/baz", + req: "/foo/baz/", + target: http.MethodGet, }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - route, tsr := r.Tree().Reverse(http.MethodGet, tc.path) - if tc.want != "" { - require.NotNil(t, route) - assert.Equal(t, tc.want, route.Path()) - assert.Equal(t, tc.wantTsr, tsr) - return + for _, method := range tc.methods { + require.NoError(t, onlyError(r.Tree().Handle(method, tc.path, emptyHandler))) } - assert.Nil(t, route) + req := httptest.NewRequest(tc.target, tc.req, nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + assert.Equal(t, http.StatusNotFound, w.Code) }) } } -func TestTree_RouteWithIgnoreTrailingSlashEnable(t *testing.T) { - routes := []string{ - "/foo/bar", - "/welcome/{name}/", - "/users/uid_{id}", - } +func TestRouterWithMethodNotAllowedHandler(t *testing.T) { + f := New(WithNoMethodHandler(func(c Context) { + c.SetHeader("FOO", "BAR") + c.Writer().WriteHeader(http.StatusMethodNotAllowed) + })) - r := New(WithIgnoreTrailingSlash(true)) - for _, rte := range routes { - require.NoError(t, onlyError(r.Handle(http.MethodGet, rte, emptyHandler))) - } + require.NoError(t, onlyError(f.Handle(http.MethodPost, "/foo/bar", emptyHandler))) + req := httptest.NewRequest(http.MethodGet, "/foo/bar", nil) + w := httptest.NewRecorder() + f.ServeHTTP(w, req) + assert.Equal(t, http.StatusMethodNotAllowed, w.Code) + assert.Equal(t, "POST", w.Header().Get("Allow")) + assert.Equal(t, "BAR", w.Header().Get("FOO")) +} + +func TestRouterWithAutomaticOptions(t *testing.T) { + f := New(WithAutoOptions(true)) cases := []struct { - name string - path string - want string - wantTsr bool + name string + target string + path string + want string + wantCode int + methods []string }{ { - name: "reverse static route", - path: "/foo/bar", - want: "/foo/bar", - }, - { - name: "reverse static route with tsr", - path: "/foo/bar/", - want: "/foo/bar", - wantTsr: true, + name: "system-wide requests", + target: "*", + path: "/foo", + methods: []string{"GET", "TRACE", "PUT"}, + want: "GET, PUT, TRACE, OPTIONS", + wantCode: http.StatusOK, }, { - name: "reverse params route", - path: "/welcome/fox/", - want: "/welcome/{name}/", + name: "system-wide with custom options registered", + target: "*", + path: "/foo", + methods: []string{"GET", "TRACE", "PUT", "OPTIONS"}, + want: "GET, PUT, TRACE, OPTIONS", + wantCode: http.StatusOK, }, { - name: "reverse params route with tsr", - path: "/welcome/fox", - want: "/welcome/{name}/", - wantTsr: true, + name: "system-wide requests with empty router", + target: "*", + wantCode: http.StatusNotFound, }, { - name: "reverse mid params route", - path: "/users/uid_123", - want: "/users/uid_{id}", + name: "regular option request", + target: "/foo", + path: "/foo", + methods: []string{"GET", "TRACE", "PUT"}, + want: "GET, PUT, TRACE, OPTIONS", + wantCode: http.StatusOK, }, { - name: "reverse mid params route with tsr", - path: "/users/uid_123/", - want: "/users/uid_{id}", - wantTsr: true, + name: "regular option request with handler priority", + target: "/foo", + path: "/foo", + methods: []string{"GET", "TRACE", "PUT", "OPTIONS"}, + want: "GET, OPTIONS, PUT, TRACE", + wantCode: http.StatusNoContent, }, { - name: "reverse no match", - path: "/users/fox", + name: "regular option request with no matching route", + target: "/bar", + path: "/foo", + methods: []string{"GET", "TRACE", "PUT"}, + wantCode: http.StatusNotFound, }, } + require.True(t, f.AutoOptionsEnabled()) for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - route, tsr := r.Tree().Reverse(http.MethodGet, tc.path) - if tc.want != "" { - require.NotNil(t, route) - assert.Equal(t, tc.want, route.Path()) - assert.Equal(t, tc.wantTsr, tsr) - return + for _, method := range tc.methods { + require.NoError(t, onlyError(f.Tree().Handle(method, tc.path, func(c Context) { + c.SetHeader("Allow", strings.Join(slices.Sorted(iterutil.Left(c.Tree().Iter().Reverse(c.Tree().Iter().Methods(), c.Request().URL.Path))), ", ")) + c.Writer().WriteHeader(http.StatusNoContent) + }))) } - assert.Nil(t, route) + req := httptest.NewRequest(http.MethodOptions, tc.target, nil) + w := httptest.NewRecorder() + f.ServeHTTP(w, req) + assert.Equal(t, tc.wantCode, w.Code) + assert.Equal(t, tc.want, w.Header().Get("Allow")) + // Reset + f.Swap(f.NewTree()) }) } } -func TestEncodedPath(t *testing.T) { - encodedPath := "run/cmd/S123L%2FA" - req := httptest.NewRequest(http.MethodGet, "/"+encodedPath, nil) - w := httptest.NewRecorder() - - r := New() - r.MustHandle(http.MethodGet, "/*{request}", func(c Context) { - _ = c.String(http.StatusOK, "%s", c.Param("request")) - }) - - r.ServeHTTP(w, req) - assert.Equal(t, encodedPath, w.Body.String()) -} +func TestRouterWithAutomaticOptionsAndIgnoreTsOptionEnable(t *testing.T) { + f := New(WithAutoOptions(true), WithIgnoreTrailingSlash(true)) -func TestTreeSwap(t *testing.T) { - f := New() - tree := f.NewTree() - assert.NotPanics(t, func() { - f.Swap(tree) - }) - assert.Equal(t, tree, f.Tree()) + cases := []struct { + name string + target string + path string + want string + wantCode int + methods []string + }{ + { + name: "system-wide requests", + target: "*", + path: "/foo", + methods: []string{"GET", "TRACE", "PUT"}, + want: "GET, PUT, TRACE, OPTIONS", + wantCode: http.StatusOK, + }, + { + name: "system-wide with custom options registered", + target: "*", + path: "/foo", + methods: []string{"GET", "TRACE", "PUT", "OPTIONS"}, + want: "GET, PUT, TRACE, OPTIONS", + wantCode: http.StatusOK, + }, + { + name: "system-wide requests with empty router", + target: "*", + wantCode: http.StatusNotFound, + }, + { + name: "regular option request and ignore ts", + target: "/foo/", + path: "/foo", + methods: []string{"GET", "TRACE", "PUT"}, + want: "GET, PUT, TRACE, OPTIONS", + wantCode: http.StatusOK, + }, + { + name: "regular option request with handler priority and ignore ts", + target: "/foo", + path: "/foo/", + methods: []string{"GET", "TRACE", "PUT", "OPTIONS"}, + want: "GET, OPTIONS, PUT, TRACE", + wantCode: http.StatusNoContent, + }, + { + name: "regular option request with no matching route", + target: "/bar", + path: "/foo", + methods: []string{"GET", "TRACE", "PUT"}, + wantCode: http.StatusNotFound, + }, + } - f2 := New() - assert.Panics(t, func() { - f2.Swap(tree) - }) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + for _, method := range tc.methods { + require.NoError(t, onlyError(f.Tree().Handle(method, tc.path, func(c Context) { + c.SetHeader("Allow", strings.Join(slices.Sorted(iterutil.Left(c.Tree().Iter().Reverse(c.Tree().Iter().Methods(), c.Request().URL.Path))), ", ")) + c.Writer().WriteHeader(http.StatusNoContent) + }))) + } + req := httptest.NewRequest(http.MethodOptions, tc.target, nil) + w := httptest.NewRecorder() + f.ServeHTTP(w, req) + assert.Equal(t, tc.wantCode, w.Code) + assert.Equal(t, tc.want, w.Header().Get("Allow")) + // Reset + f.Swap(f.NewTree()) + }) + } } -func TestFuzzInsertLookupParam(t *testing.T) { - // no '*', '{}' and '/' and invalid escape char - unicodeRanges := fuzz.UnicodeRanges{ - {First: 0x20, Last: 0x29}, - {First: 0x2B, Last: 0x2E}, - {First: 0x30, Last: 0x7A}, - {First: 0x7C, Last: 0x7C}, - {First: 0x7E, Last: 0x04FF}, - } +func TestRouterWithAutomaticOptionsAndIgnoreTsOptionDisable(t *testing.T) { + f := New(WithAutoOptions(true)) - tree := New().Tree() - f := fuzz.New().NilChance(0).Funcs(unicodeRanges.CustomStringFuzzFunc()) - routeFormat := "/%s/{%s}/%s/{%s}/{%s}" - reqFormat := "/%s/%s/%s/%s/%s" - for i := 0; i < 2000; i++ { - var s1, e1, s2, e2, e3 string - f.Fuzz(&s1) - f.Fuzz(&e1) - f.Fuzz(&s2) - f.Fuzz(&e2) - f.Fuzz(&e3) - if s1 == "" || s2 == "" || e1 == "" || e2 == "" || e3 == "" { - continue - } - path := fmt.Sprintf(routeFormat, s1, e1, s2, e2, e3) - if err := tree.insert(http.MethodGet, tree.newRoute(path, emptyHandler), 3); err == nil { - nds := *tree.nodes.Load() + cases := []struct { + name string + target string + path string + wantCode int + methods []string + }{ + { + name: "regular option request and ignore ts", + target: "/foo/", + path: "/foo", + methods: []string{"GET", "TRACE", "PUT"}, + wantCode: http.StatusNotFound, + }, + { + name: "regular option request with handler priority and ignore ts", + target: "/foo", + path: "/foo/", + methods: []string{"GET", "TRACE", "PUT", "OPTIONS"}, + wantCode: http.StatusNotFound, + }, + } - c := newTestContextTree(tree) - n, tsr := tree.lookup(nds[0].children[0].Load(), fmt.Sprintf(reqFormat, s1, "xxxx", s2, "xxxx", "xxxx"), c, false) - require.NotNil(t, n) - require.NotNil(t, n.route) - assert.False(t, tsr) - assert.Equal(t, fmt.Sprintf(routeFormat, s1, e1, s2, e2, e3), n.route.path) - assert.Equal(t, "xxxx", c.Param(e1)) - assert.Equal(t, "xxxx", c.Param(e2)) - assert.Equal(t, "xxxx", c.Param(e3)) - } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + for _, method := range tc.methods { + require.NoError(t, onlyError(f.Tree().Handle(method, tc.path, func(c Context) { + c.SetHeader("Allow", strings.Join(slices.Sorted(iterutil.Left(c.Tree().Iter().Reverse(c.Tree().Iter().Methods(), c.Request().URL.Path))), ", ")) + c.Writer().WriteHeader(http.StatusNoContent) + }))) + } + req := httptest.NewRequest(http.MethodOptions, tc.target, nil) + w := httptest.NewRecorder() + f.ServeHTTP(w, req) + assert.Equal(t, tc.wantCode, w.Code) + // Reset + f.Swap(f.NewTree()) + }) } } -func TestFuzzInsertNoPanics(t *testing.T) { - f := fuzz.New().NilChance(0).NumElements(5000, 10000) - tree := New().Tree() +func TestRouterWithOptionsHandler(t *testing.T) { + f := New(WithOptionsHandler(func(c Context) { + assert.Equal(t, "", c.Path()) + assert.Empty(t, slices.Collect(c.Params())) + c.Writer().WriteHeader(http.StatusNoContent) + })) - routes := make(map[string]struct{}) - f.Fuzz(&routes) + require.NoError(t, onlyError(f.Handle(http.MethodGet, "/foo/{bar}", emptyHandler))) + require.NoError(t, onlyError(f.Handle(http.MethodPost, "/foo/{bar}", emptyHandler))) - for rte := range routes { - if rte == "" { - continue + req := httptest.NewRequest(http.MethodOptions, "/foo/bar", nil) + w := httptest.NewRecorder() + f.ServeHTTP(w, req) + assert.Equal(t, http.StatusNoContent, w.Code) + assert.Equal(t, "GET, POST, OPTIONS", w.Header().Get("Allow")) +} + +func TestDefaultOptions(t *testing.T) { + m := MiddlewareFunc(func(next HandlerFunc) HandlerFunc { + return func(c Context) { + next(c) } - require.NotPanicsf(t, func() { - _ = tree.insert(http.MethodGet, tree.newRoute(rte, emptyHandler), 0) - }, fmt.Sprintf("rte: %s", rte)) - } + }) + r := New(WithMiddleware(m), DefaultOptions()) + assert.Equal(t, reflect.ValueOf(m).Pointer(), reflect.ValueOf(r.mws[2].m).Pointer()) + assert.True(t, r.handleOptions) } -func TestFuzzInsertLookupUpdateAndDelete(t *testing.T) { - // no '*' and '{}' and invalid escape char - unicodeRanges := fuzz.UnicodeRanges{ - {First: 0x20, Last: 0x29}, - {First: 0x2B, Last: 0x7A}, - {First: 0x7C, Last: 0x7C}, - {First: 0x7E, Last: 0x04FF}, - } +func TestWithScopedMiddleware(t *testing.T) { + called := false + m := MiddlewareFunc(func(next HandlerFunc) HandlerFunc { + return func(c Context) { + called = true + next(c) + } + }) - f := fuzz.New().NilChance(0).NumElements(1000, 2000).Funcs(unicodeRanges.CustomStringFuzzFunc()) - tree := New().Tree() + r := New(WithMiddlewareFor(NoRouteHandler, m)) + require.NoError(t, onlyError(r.Handle(http.MethodGet, "/foo/bar", emptyHandler))) - routes := make(map[string]struct{}) - f.Fuzz(&routes) + req := httptest.NewRequest(http.MethodGet, "/foo/bar", nil) + w := httptest.NewRecorder() - for rte := range routes { - path := "/" + rte - err := tree.insert(http.MethodGet, tree.newRoute(path, emptyHandler), 0) - require.NoError(t, err) - } + r.ServeHTTP(w, req) + assert.False(t, called) + req.URL.Path = "/foo" + r.ServeHTTP(w, req) + assert.True(t, called) +} - it := tree.Iter() - countPath := len(slices.Collect(iterutil.Right(it.All()))) - assert.Equal(t, len(routes), countPath) +func TestUpdateWithMiddleware(t *testing.T) { + called := false + m := MiddlewareFunc(func(next HandlerFunc) HandlerFunc { + return func(c Context) { + called = true + next(c) + } + }) + f := New(WithMiddleware(Recovery())) + f.MustHandle(http.MethodGet, "/foo", emptyHandler) + req := httptest.NewRequest(http.MethodGet, "/foo", nil) + w := httptest.NewRecorder() - for rte := range routes { - nds := *tree.nodes.Load() - c := newTestContextTree(tree) - n, tsr := tree.lookup(nds[0].children[0].Load(), "/"+rte, c, true) - require.NotNilf(t, n, "route /%s", rte) - require.NotNilf(t, n.route, "route /%s", rte) - require.Falsef(t, tsr, "tsr: %t", tsr) - require.Truef(t, n.isLeaf(), "route /%s", rte) - require.Equal(t, "/"+rte, n.route.path) - path := "/" + rte - require.NoError(t, tree.update(http.MethodGet, tree.newRoute(path, emptyHandler))) - } + // Add middleware + require.NoError(t, onlyError(f.Update(http.MethodGet, "/foo", emptyHandler, WithMiddleware(m)))) + f.ServeHTTP(w, req) + assert.True(t, called) + called = false - for rte := range routes { - deleted := tree.remove(http.MethodGet, "/"+rte) - require.True(t, deleted) - } + rte := f.Tree().Route(http.MethodGet, "/foo") + rte.Handle(newTestContextTree(f.Tree())) + assert.False(t, called) + called = false - it = tree.Iter() - countPath = len(slices.Collect(iterutil.Right(it.All()))) - assert.Equal(t, 0, countPath) -} + rte.HandleMiddleware(newTestContextTree(f.Tree())) + assert.True(t, called) + called = false -func TestDataRace(t *testing.T) { - var wg sync.WaitGroup - start, wait := atomicSync() + // Remove middleware + require.NoError(t, onlyError(f.Update(http.MethodGet, "/foo", emptyHandler))) + f.ServeHTTP(w, req) + assert.False(t, called) + called = false - h := HandlerFunc(func(c Context) {}) - newH := HandlerFunc(func(c Context) {}) + rte = f.Tree().Route(http.MethodGet, "/foo") + rte.Handle(newTestContextTree(f.Tree())) + assert.False(t, called) + called = false - r := New() + rte = f.Tree().Route(http.MethodGet, "/foo") + rte.HandleMiddleware(newTestContextTree(f.Tree())) + assert.False(t, called) +} - w := new(mockResponseWriter) +func TestRouteMiddleware(t *testing.T) { + var c0, c1, c2 bool + m0 := MiddlewareFunc(func(next HandlerFunc) HandlerFunc { + return func(c Context) { + c0 = true + next(c) + } + }) - wg.Add(len(githubAPI) * 3) - for _, rte := range githubAPI { - go func(method, route string) { - wait() - defer wg.Done() - tree := r.Tree() - tree.Lock() - defer tree.Unlock() - if tree.Has(method, route) { - assert.NoError(t, onlyError(tree.Update(method, route, h))) - return - } - assert.NoError(t, onlyError(tree.Handle(method, route, h))) - // assert.NoError(t, r.Handle("PING", route, h)) - }(rte.method, rte.path) - - go func(method, route string) { - wait() - defer wg.Done() - tree := r.Tree() - tree.Lock() - defer tree.Unlock() - if tree.Has(method, route) { - assert.NoError(t, tree.Delete(method, route)) - return - } - assert.NoError(t, onlyError(tree.Handle(method, route, newH))) - }(rte.method, rte.path) - - go func(method, route string) { - wait() - req := httptest.NewRequest(method, route, nil) - r.ServeHTTP(w, req) - wg.Done() - }(rte.method, rte.path) - } + m1 := MiddlewareFunc(func(next HandlerFunc) HandlerFunc { + return func(c Context) { + c1 = true + next(c) + } + }) - time.Sleep(500 * time.Millisecond) - start() - wg.Wait() -} + m2 := MiddlewareFunc(func(next HandlerFunc) HandlerFunc { + return func(c Context) { + c2 = true + next(c) + } + }) + f := New(WithMiddleware(m0)) + f.MustHandle(http.MethodGet, "/1", emptyHandler, WithMiddleware(m1)) + f.MustHandle(http.MethodGet, "/2", emptyHandler, WithMiddleware(m2)) -func TestTree_RaceDetector(t *testing.T) { - var wg sync.WaitGroup - start, wait := atomicSync() - var raceCount atomic.Uint32 + req := httptest.NewRequest(http.MethodGet, "/1", nil) + w := httptest.NewRecorder() - tree := New().Tree() + f.ServeHTTP(w, req) + assert.True(t, c0) + assert.True(t, c1) + assert.False(t, c2) + c0, c1, c2 = false, false, false - wg.Add(len(staticRoutes) * 3) - for _, rte := range staticRoutes { - go func() { - wait() - defer func() { - if v := recover(); v != nil { - raceCount.Add(1) - assert.ErrorIs(t, v.(error), ErrConcurrentAccess) - } - wg.Done() - }() - _ = tree.insert(rte.method, &Route{path: rte.path}, 0) - }() + req.URL.Path = "/2" + f.ServeHTTP(w, req) + assert.True(t, c0) + assert.False(t, c1) + assert.True(t, c2) - go func() { - wait() - defer func() { - if v := recover(); v != nil { - raceCount.Add(1) - assert.ErrorIs(t, v.(error), ErrConcurrentAccess) - } - wg.Done() - }() - _ = tree.update(rte.method, &Route{path: rte.path}) - }() + c0, c1, c2 = false, false, false + rte1 := f.Tree().Route(http.MethodGet, "/1") + require.NotNil(t, rte1) + rte1.Handle(newTestContextTree(f.Tree())) + assert.False(t, c0) + assert.False(t, c1) + assert.False(t, c2) + c0, c1, c2 = false, false, false - go func() { - wait() - defer func() { - if v := recover(); v != nil { - raceCount.Add(1) - assert.ErrorIs(t, v.(error), ErrConcurrentAccess) - } - wg.Done() - }() - tree.remove(rte.method, rte.path) - }() - } + rte1.HandleMiddleware(newTestContextTree(f.Tree())) + assert.False(t, c0) + assert.True(t, c1) + assert.False(t, c2) + c0, c1, c2 = false, false, false - time.Sleep(500 * time.Millisecond) - start() - wg.Wait() - assert.GreaterOrEqual(t, raceCount.Load(), uint32(1)) + rte2 := f.Tree().Route(http.MethodGet, "/2") + require.NotNil(t, rte2) + rte2.HandleMiddleware(newTestContextTree(f.Tree())) + assert.False(t, c0) + assert.False(t, c1) + assert.True(t, c2) } -func TestConcurrentRequestHandling(t *testing.T) { - r := New() - - // /repos/{owner}/{repo}/keys - h1 := HandlerFunc(func(c Context) { - assert.Equal(t, "john", c.Param("owner")) - assert.Equal(t, "fox", c.Param("repo")) - _ = c.String(200, c.Path()) - }) - - // /repos/{owner}/{repo}/contents/*{path} - h2 := HandlerFunc(func(c Context) { - assert.Equal(t, "alex", c.Param("owner")) - assert.Equal(t, "vault", c.Param("repo")) - assert.Equal(t, "file.txt", c.Param("path")) - _ = c.String(200, c.Path()) - }) - - // /users/{user}/received_events/public - h3 := HandlerFunc(func(c Context) { - assert.Equal(t, "go", c.Param("user")) - _ = c.String(200, c.Path()) - }) +func TestMiddlewareLength(t *testing.T) { + f := New(DefaultOptions()) + r := f.MustHandle(http.MethodGet, "/", emptyHandler, WithMiddleware(Recovery(), Logger())) + assert.Len(t, f.mws, 2) + assert.Len(t, r.mws, 4) +} - require.NoError(t, onlyError(r.Handle(http.MethodGet, "/repos/{owner}/{repo}/keys", h1))) - require.NoError(t, onlyError(r.Handle(http.MethodGet, "/repos/{owner}/{repo}/contents/*{path}", h2))) - require.NoError(t, onlyError(r.Handle(http.MethodGet, "/users/{user}/received_events/public", h3))) +func TestWithNotFoundHandler(t *testing.T) { + notFound := func(c Context) { + _ = c.String(http.StatusNotFound, "NOT FOUND\n") + } - r1 := httptest.NewRequest(http.MethodGet, "/repos/john/fox/keys", nil) - r2 := httptest.NewRequest(http.MethodGet, "/repos/alex/vault/contents/file.txt", nil) - r3 := httptest.NewRequest(http.MethodGet, "/users/go/received_events/public", nil) + f := New(WithNoRouteHandler(notFound)) + require.NoError(t, onlyError(f.Handle(http.MethodGet, "/foo", emptyHandler))) - var wg sync.WaitGroup - wg.Add(300) - start, wait := atomicSync() - for i := 0; i < 100; i++ { - go func() { - defer wg.Done() - wait() - w := httptest.NewRecorder() - r.ServeHTTP(w, r1) - assert.Equal(t, "/repos/{owner}/{repo}/keys", w.Body.String()) - }() + req := httptest.NewRequest(http.MethodGet, "/foo/bar", nil) + w := httptest.NewRecorder() - go func() { - defer wg.Done() - wait() - w := httptest.NewRecorder() - r.ServeHTTP(w, r2) - assert.Equal(t, "/repos/{owner}/{repo}/contents/*{path}", w.Body.String()) - }() + f.ServeHTTP(w, req) + assert.Equal(t, http.StatusNotFound, w.Code) + assert.Equal(t, "NOT FOUND\n", w.Body.String()) +} - go func() { - defer wg.Done() - wait() - w := httptest.NewRecorder() - r.ServeHTTP(w, r3) - assert.Equal(t, "/users/{user}/received_events/public", w.Body.String()) - }() +func TestRouter_Lookup(t *testing.T) { + rx := regexp.MustCompile("({|\\*{)[A-z]+[}]") + f := New() + for _, rte := range githubAPI { + require.NoError(t, onlyError(f.Handle(rte.method, rte.path, emptyHandler))) } - start() - wg.Wait() -} + for _, rte := range githubAPI { + req := httptest.NewRequest(rte.method, rte.path, nil) + route, cc, _ := f.Lookup(newResponseWriter(mockResponseWriter{}), req) + require.NotNil(t, cc) + require.NotNil(t, route) + assert.Equal(t, rte.path, route.Path()) -func atomicSync() (start func(), wait func()) { - var n int32 + matches := rx.FindAllString(rte.path, -1) + for _, match := range matches { + var key string + if strings.HasPrefix(match, "*") { + key = match[2 : len(match)-1] + } else { + key = match[1 : len(match)-1] + } + value := match + assert.Equal(t, value, cc.Param(key)) + } - start = func() { - atomic.StoreInt32(&n, 1) + cc.Close() } - wait = func() { - for atomic.LoadInt32(&n) != 1 { - time.Sleep(1 * time.Microsecond) - } - } + // No method match + req := httptest.NewRequest("ANY", "/bar", nil) + route, cc, _ := f.Lookup(newResponseWriter(mockResponseWriter{}), req) + assert.Nil(t, route) + assert.Nil(t, cc) - return + // No path match + req = httptest.NewRequest(http.MethodGet, "/bar", nil) + route, cc, _ = f.Lookup(newResponseWriter(mockResponseWriter{}), req) + assert.Nil(t, route) + assert.Nil(t, cc) } -func TestNode_String(t *testing.T) { - f := New() - require.NoError(t, onlyError(f.Handle(http.MethodGet, "/foo/{bar}/*{baz}", emptyHandler))) - tree := f.Tree() - nds := *tree.nodes.Load() +func TestTree_Has(t *testing.T) { + routes := []string{ + "/foo/bar", + "/welcome/{name}", + "/users/uid_{id}", + "/john/doe/", + } - want := `path: GET - path: /foo/{bar}/*{baz} [leaf=/foo/{bar}/*{baz}] [bar (10), baz (-1)]` - assert.Equal(t, want, strings.TrimSuffix(nds[0].String(), "\n")) -} + r := New() + for _, rte := range routes { + require.NoError(t, onlyError(r.Handle(http.MethodGet, rte, emptyHandler))) + } -func TestFixTrailingSlash(t *testing.T) { - assert.Equal(t, "/foo/", FixTrailingSlash("/foo")) - assert.Equal(t, "/foo", FixTrailingSlash("/foo/")) - assert.Equal(t, "/", FixTrailingSlash("")) + cases := []struct { + name string + path string + want bool + }{ + { + name: "strict match static route", + path: "/foo/bar", + want: true, + }, + { + name: "strict match static route", + path: "/john/doe/", + want: true, + }, + { + name: "no match static route (tsr)", + path: "/foo/bar/", + }, + { + name: "no match static route (tsr)", + path: "/john/doe", + }, + { + name: "strict match route params", + path: "/welcome/{name}", + want: true, + }, + { + name: "no match route params", + path: "/welcome/fox", + }, + { + name: "strict match mid route params", + path: "/users/uid_{id}", + want: true, + }, + { + name: "no match mid route params", + path: "/users/uid_123", + }, + } + + tree := r.Tree() + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, tree.Has(http.MethodGet, tc.path)) + }) + } } -// This example demonstrates how to create a simple router using the default options, -// which include the Recovery and Logger middleware. A basic route is defined, along with a -// custom middleware to log the request metrics. -func ExampleNew() { - // Create a new router with default options, which include the Recovery and Logger middleware - r := New(DefaultOptions()) +func TestTree_Route(t *testing.T) { + routes := []string{ + "/foo/bar", + "/welcome/{name}", + "/users/uid_{id}", + } - // Define a custom middleware to measure the time taken for request processing and - // log the URL, route, time elapsed, and status code. - metrics := func(next HandlerFunc) HandlerFunc { - return func(c Context) { - start := time.Now() - next(c) - log.Printf("url=%s; route=%s; time=%d; status=%d", c.Request().URL, c.Path(), time.Since(start), c.Writer().Status()) - } + r := New() + for _, rte := range routes { + require.NoError(t, onlyError(r.Handle(http.MethodGet, rte, emptyHandler))) } - // Define a route with the path "/hello/{name}", apply the custom "metrics" middleware, - // and set a simple handler that greets the user by their name. - r.MustHandle(http.MethodGet, "/hello/{name}", metrics(func(c Context) { - _ = c.String(200, "Hello %s\n", c.Param("name")) - })) + cases := []struct { + name string + path string + want string + wantTsr bool + }{ + { + name: "reverse static route", + path: "/foo/bar", + want: "/foo/bar", + }, + { + name: "reverse static route with tsr disable", + path: "/foo/bar/", + want: "/foo/bar", + wantTsr: true, + }, + { + name: "reverse params route", + path: "/welcome/fox", + want: "/welcome/{name}", + }, + { + name: "reverse mid params route", + path: "/users/uid_123", + want: "/users/uid_{id}", + }, + { + name: "reverse no match", + path: "/users/fox", + }, + } - // Start the HTTP server using the router as the handler and listen on port 8080 - log.Fatalln(http.ListenAndServe(":8080", r)) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + route, tsr := r.Tree().Reverse(http.MethodGet, tc.path) + if tc.want != "" { + require.NotNil(t, route) + assert.Equal(t, tc.want, route.Path()) + assert.Equal(t, tc.wantTsr, tsr) + return + } + assert.Nil(t, route) + }) + } } -// This example demonstrates how to register a global middleware that will be -// applied to all routes. -func ExampleWithMiddleware() { - // Define a custom middleware to measure the time taken for request processing and - // log the URL, route, time elapsed, and status code. - metrics := func(next HandlerFunc) HandlerFunc { - return func(c Context) { - start := time.Now() - next(c) - log.Printf( - "url=%s; route=%s; time=%d; status=%d", - c.Request().URL, - c.Path(), - time.Since(start), - c.Writer().Status(), - ) - } +func TestTree_RouteWithIgnoreTrailingSlashEnable(t *testing.T) { + routes := []string{ + "/foo/bar", + "/welcome/{name}/", + "/users/uid_{id}", } - f := New(WithMiddleware(metrics)) - - f.MustHandle(http.MethodGet, "/hello/{name}", func(c Context) { - _ = c.String(200, "Hello %s\n", c.Param("name")) - }) -} + r := New(WithIgnoreTrailingSlash(true)) + for _, rte := range routes { + require.NoError(t, onlyError(r.Handle(http.MethodGet, rte, emptyHandler))) + } -// This example demonstrates some important considerations when using the Tree API. -func ExampleRouter_Tree() { - r := New() + cases := []struct { + name string + path string + want string + wantTsr bool + }{ + { + name: "reverse static route", + path: "/foo/bar", + want: "/foo/bar", + }, + { + name: "reverse static route with tsr", + path: "/foo/bar/", + want: "/foo/bar", + wantTsr: true, + }, + { + name: "reverse params route", + path: "/welcome/fox/", + want: "/welcome/{name}/", + }, + { + name: "reverse params route with tsr", + path: "/welcome/fox", + want: "/welcome/{name}/", + wantTsr: true, + }, + { + name: "reverse mid params route", + path: "/users/uid_123", + want: "/users/uid_{id}", + }, + { + name: "reverse mid params route with tsr", + path: "/users/uid_123/", + want: "/users/uid_{id}", + wantTsr: true, + }, + { + name: "reverse no match", + path: "/users/fox", + }, + } - // Each tree as its own sync.Mutex that is used to lock write on the tree. Since the router tree may be swapped at - // any given time, you MUST always copy the pointer locally, This ensures that you do not inadvertently cause a - // deadlock by locking/unlocking the wrong tree. - tree := r.Tree() - upsert := func(method, path string, handler HandlerFunc) (*Route, error) { - tree.Lock() - defer tree.Unlock() - if tree.Has(method, path) { - return tree.Update(method, path, handler) - } - return tree.Handle(method, path, handler) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + route, tsr := r.Tree().Reverse(http.MethodGet, tc.path) + if tc.want != "" { + require.NotNil(t, route) + assert.Equal(t, tc.want, route.Path()) + assert.Equal(t, tc.wantTsr, tsr) + return + } + assert.Nil(t, route) + }) } +} - _, _ = upsert(http.MethodGet, "/foo/bar", func(c Context) { - // Note the tree accessible from fox.Context is already a local copy so the golden rule above does not apply. - c.Tree().Lock() - defer c.Tree().Unlock() - _ = c.String(200, "foo bar") +func TestEncodedPath(t *testing.T) { + encodedPath := "run/cmd/S123L%2FA" + req := httptest.NewRequest(http.MethodGet, "/"+encodedPath, nil) + w := httptest.NewRecorder() + + r := New() + r.MustHandle(http.MethodGet, "/*{request}", func(c Context) { + _ = c.String(http.StatusOK, "%s", c.Param("request")) }) - // Bad, instead make a local copy of the tree! - _ = func(method, path string, handler HandlerFunc) (*Route, error) { - r.Tree().Lock() - defer r.Tree().Unlock() - if r.Tree().Has(method, path) { - return r.Tree().Update(method, path, handler) - } - return r.Tree().Handle(method, path, handler) - } + r.ServeHTTP(w, req) + assert.Equal(t, encodedPath, w.Body.String()) } -// This example demonstrates how to create a custom middleware that cleans the request path and performs a manual -// lookup on the tree. If the cleaned path matches a registered route, the client is redirected to the valid path. -func ExampleRouter_Lookup() { - redirectFixedPath := MiddlewareFunc(func(next HandlerFunc) HandlerFunc { - return func(c Context) { - req := c.Request() - target := req.URL.Path - cleanedPath := CleanPath(target) +func TestTreeSwap(t *testing.T) { + f := New() + tree := f.NewTree() + assert.NotPanics(t, func() { + f.Swap(tree) + }) + assert.Equal(t, tree, f.Tree()) - // Nothing to clean, call next handler. - if cleanedPath == target { - next(c) - return - } + f2 := New() + assert.Panics(t, func() { + f2.Swap(tree) + }) +} - req.URL.Path = cleanedPath - route, cc, tsr := c.Fox().Lookup(c.Writer(), req) - if route != nil { - defer cc.Close() +func TestFuzzInsertLookupParam(t *testing.T) { + // no '*', '{}' and '/' and invalid escape char + unicodeRanges := fuzz.UnicodeRanges{ + {First: 0x20, Last: 0x29}, + {First: 0x2B, Last: 0x2E}, + {First: 0x30, Last: 0x7A}, + {First: 0x7C, Last: 0x7C}, + {First: 0x7E, Last: 0x04FF}, + } - code := http.StatusMovedPermanently - if req.Method != http.MethodGet { - code = http.StatusPermanentRedirect - } + tree := New().Tree() + f := fuzz.New().NilChance(0).Funcs(unicodeRanges.CustomStringFuzzFunc()) + routeFormat := "/%s/{%s}/%s/{%s}/{%s}" + reqFormat := "/%s/%s/%s/%s/%s" + for i := 0; i < 2000; i++ { + var s1, e1, s2, e2, e3 string + f.Fuzz(&s1) + f.Fuzz(&e1) + f.Fuzz(&s2) + f.Fuzz(&e2) + f.Fuzz(&e3) + if s1 == "" || s2 == "" || e1 == "" || e2 == "" || e3 == "" { + continue + } + path := fmt.Sprintf(routeFormat, s1, e1, s2, e2, e3) + if err := tree.insert(http.MethodGet, tree.newRoute(path, emptyHandler), 3); err == nil { + nds := *tree.nodes.Load() - // Redirect the client if direct match or indirect match. - if !tsr || route.IgnoreTrailingSlashEnabled() { - if err := c.Redirect(code, cleanedPath); err != nil { - // Only if not in the range 300..308, so not possible here! - panic(err) - } - return - } + c := newTestContextTree(tree) + n, tsr := tree.lookup(nds[0].children[0].Load(), fmt.Sprintf(reqFormat, s1, "xxxx", s2, "xxxx", "xxxx"), c, false) + require.NotNil(t, n) + require.NotNil(t, n.route) + assert.False(t, tsr) + assert.Equal(t, fmt.Sprintf(routeFormat, s1, e1, s2, e2, e3), n.route.path) + assert.Equal(t, "xxxx", c.Param(e1)) + assert.Equal(t, "xxxx", c.Param(e2)) + assert.Equal(t, "xxxx", c.Param(e3)) + } + } +} - // Add or remove an extra trailing slash and redirect the client. - if route.RedirectTrailingSlashEnabled() { - if err := c.Redirect(code, FixTrailingSlash(cleanedPath)); err != nil { - // Only if not in the range 300..308, so not possible here - panic(err) - } - return - } - } +func TestFuzzInsertNoPanics(t *testing.T) { + f := fuzz.New().NilChance(0).NumElements(5000, 10000) + tree := New().Tree() - // rollback to the original path before calling the - // next handler or middleware. - req.URL.Path = target - next(c) + routes := make(map[string]struct{}) + f.Fuzz(&routes) + + for rte := range routes { + if rte == "" { + continue } - }) + require.NotPanicsf(t, func() { + _ = tree.insert(http.MethodGet, tree.newRoute(rte, emptyHandler), 0) + }, fmt.Sprintf("rte: %s", rte)) + } +} - f := New( - // Register the middleware for the NoRouteHandler scope. - WithMiddlewareFor(NoRouteHandler|NoMethodHandler, redirectFixedPath), - ) +func TestFuzzInsertLookupUpdateAndDelete(t *testing.T) { + // no '*' and '{}' and invalid escape char + unicodeRanges := fuzz.UnicodeRanges{ + {First: 0x20, Last: 0x29}, + {First: 0x2B, Last: 0x7A}, + {First: 0x7C, Last: 0x7C}, + {First: 0x7E, Last: 0x04FF}, + } - f.MustHandle(http.MethodGet, "/hello/{name}", func(c Context) { - _ = c.String(200, "Hello %s\n", c.Param("name")) - }) -} + f := fuzz.New().NilChance(0).NumElements(1000, 2000).Funcs(unicodeRanges.CustomStringFuzzFunc()) + tree := New().Tree() -// This example demonstrates how to do a reverse lookup on the tree. -func ExampleTree_Reverse() { - f := New() - f.MustHandle(http.MethodGet, "/hello/{name}", emptyHandler) + routes := make(map[string]struct{}) + f.Fuzz(&routes) - tree := f.Tree() - route, _ := tree.Reverse(http.MethodGet, "/hello/fox") - fmt.Println(route.Path()) // /hello/{name} -} + for rte := range routes { + path := "/" + rte + err := tree.insert(http.MethodGet, tree.newRoute(path, emptyHandler), 0) + require.NoError(t, err) + } -// This example demonstrates how to check if a given route is registered in the tree. -func ExampleTree_Has() { - f := New() - f.MustHandle(http.MethodGet, "/hello/{name}", emptyHandler) + it := tree.Iter() + countPath := len(slices.Collect(iterutil.Right(it.All()))) + assert.Equal(t, len(routes), countPath) - tree := f.Tree() - exist := tree.Has(http.MethodGet, "/hello/{name}") - fmt.Println(exist) // true + for rte := range routes { + nds := *tree.nodes.Load() + c := newTestContextTree(tree) + n, tsr := tree.lookup(nds[0].children[0].Load(), "/"+rte, c, true) + require.NotNilf(t, n, "route /%s", rte) + require.NotNilf(t, n.route, "route /%s", rte) + require.Falsef(t, tsr, "tsr: %t", tsr) + require.Truef(t, n.isLeaf(), "route /%s", rte) + require.Equal(t, "/"+rte, n.route.path) + path := "/" + rte + require.NoError(t, tree.update(http.MethodGet, tree.newRoute(path, emptyHandler))) + } + + for rte := range routes { + deleted := tree.remove(http.MethodGet, "/"+rte) + require.True(t, deleted) + } + + it = tree.Iter() + countPath = len(slices.Collect(iterutil.Right(it.All()))) + assert.Equal(t, 0, countPath) } -func onlyError[T any](_ T, err error) error { - return err +func TestDataRace(t *testing.T) { + var wg sync.WaitGroup + start, wait := atomicSync() + + h := HandlerFunc(func(c Context) {}) + newH := HandlerFunc(func(c Context) {}) + + r := New() + + w := new(mockResponseWriter) + + wg.Add(len(githubAPI) * 3) + for _, rte := range githubAPI { + go func(method, route string) { + wait() + defer wg.Done() + tree := r.Tree() + tree.Lock() + defer tree.Unlock() + if tree.Has(method, route) { + assert.NoError(t, onlyError(tree.Update(method, route, h))) + return + } + assert.NoError(t, onlyError(tree.Handle(method, route, h))) + // assert.NoError(t, r.Handle("PING", route, h)) + }(rte.method, rte.path) + + go func(method, route string) { + wait() + defer wg.Done() + tree := r.Tree() + tree.Lock() + defer tree.Unlock() + if tree.Has(method, route) { + assert.NoError(t, tree.Delete(method, route)) + return + } + assert.NoError(t, onlyError(tree.Handle(method, route, newH))) + }(rte.method, rte.path) + + go func(method, route string) { + wait() + req := httptest.NewRequest(method, route, nil) + r.ServeHTTP(w, req) + wg.Done() + }(rte.method, rte.path) + } + + time.Sleep(500 * time.Millisecond) + start() + wg.Wait() } -func TestInfixWildcard(t *testing.T) { - cases := []struct { - name string - routes []string - path string - wantPath string - wantTsr bool - wantParams []Param - }{ - { - name: "simple infix wildcard", - routes: []string{"/foo/*{args}/bar"}, - path: "/foo/a/bar", - wantPath: "/foo/*{args}/bar", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "a", - }, - }, - }, - { - name: "simple infix wildcard with route char", - routes: []string{"/foo/*{args}/bar"}, - path: "/foo/*{args}/bar", - wantPath: "/foo/*{args}/bar", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "*{args}", - }, - }, - }, - { - name: "simple infix wildcard with multi segment and route char", - routes: []string{"/foo/*{args}/bar"}, - path: "/foo/*{args}/b/c/bar", - wantPath: "/foo/*{args}/bar", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "*{args}/b/c", - }, - }, - }, - { - name: "simple infix inflight wildcard", - routes: []string{"/foo/z*{args}/bar"}, - path: "/foo/za/bar", - wantPath: "/foo/z*{args}/bar", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "a", - }, - }, - }, - { - name: "simple infix inflight wildcard with route char", - routes: []string{"/foo/z*{args}/bar"}, - path: "/foo/z*{args}/bar", - wantPath: "/foo/z*{args}/bar", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "*{args}", - }, - }, - }, - { - name: "simple infix inflight wildcard with multi segment", - routes: []string{"/foo/z*{args}/bar"}, - path: "/foo/za/b/c/bar", - wantPath: "/foo/z*{args}/bar", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "a/b/c", - }, - }, - }, - { - name: "simple infix inflight wildcard with multi segment and route char", - routes: []string{"/foo/z*{args}/bar"}, - path: "/foo/z*{args}/b/c/bar", - wantPath: "/foo/z*{args}/bar", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "*{args}/b/c", - }, - }, - }, - { - name: "simple infix inflight wildcard long", - routes: []string{"/foo/xyz*{args}/bar"}, - path: "/foo/xyza/bar", - wantPath: "/foo/xyz*{args}/bar", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "a", - }, - }, - }, - { - name: "simple infix inflight wildcard with multi segment long", - routes: []string{"/foo/xyz*{args}/bar"}, - path: "/foo/xyza/b/c/bar", - wantPath: "/foo/xyz*{args}/bar", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "a/b/c", - }, - }, - }, - { - name: "overlapping infix and suffix wildcard match infix", - routes: []string{"/foo/*{args}", "/foo/*{args}/bar"}, - path: "/foo/a/b/c/bar", - wantPath: "/foo/*{args}/bar", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "a/b/c", - }, - }, - }, - { - name: "overlapping infix and suffix wildcard match suffix", - routes: []string{"/foo/*{args}", "/foo/*{args}/bar"}, - path: "/foo/a/b/c/baz", - wantPath: "/foo/*{args}", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "a/b/c/baz", - }, - }, - }, - { - name: "overlapping infix and suffix wildcard match suffix", - routes: []string{"/foo/*{args}", "/foo/*{args}/bar"}, - path: "/foo/a/b/c/barito", - wantPath: "/foo/*{args}", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "a/b/c/barito", - }, - }, - }, - { - name: "overlapping infix suffix wildcard and param match infix", - routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/bar"}, - path: "/foo/a/b/c/bar", - wantPath: "/foo/*{args}/bar", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "a/b/c", - }, - }, - }, - { - name: "overlapping infix suffix wildcard and param match suffix", - routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/bar"}, - path: "/foo/a/b/c/bili", - wantPath: "/foo/*{args}", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "a/b/c/bili", - }, - }, - }, - { - name: "overlapping infix suffix wildcard and param match infix", - routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}"}, - path: "/foo/a/bar", - wantPath: "/foo/*{args}/bar", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "a", - }, - }, - }, - { - name: "overlapping infix suffix wildcard and param match param", - routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/bar"}, - path: "/foo/a/bar", - wantPath: "/foo/{ps}/bar", - wantTsr: false, - wantParams: Params{ - { - Key: "ps", - Value: "a", - }, - }, - }, - { - name: "overlapping infix suffix wildcard and param match suffix", - routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}"}, - path: "/foo/a/bili", - wantPath: "/foo/*{args}", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "a/bili", - }, - }, - }, - { - name: "overlapping infix suffix wildcard and param match param", - routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}"}, - path: "/foo/a", - wantPath: "/foo/{ps}", - wantTsr: false, - wantParams: Params{ - { - Key: "ps", - Value: "a", - }, - }, - }, - { - name: "overlapping infix suffix wildcard and param match param with ts", - routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/"}, - path: "/foo/a/", - wantPath: "/foo/{ps}/", - wantTsr: false, - wantParams: Params{ - { - Key: "ps", - Value: "a", - }, - }, - }, - { - name: "overlapping infix suffix wildcard and param match suffix without ts", - routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/"}, - path: "/foo/a", - wantPath: "/foo/*{args}", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "a", - }, - }, - }, - { - name: "overlapping infix suffix wildcard and param match suffix without ts", - routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}"}, - path: "/foo/a/", - wantPath: "/foo/*{args}", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "a/", - }, - }, - }, - { - name: "overlapping infix inflight suffix wildcard and param match param", - routes: []string{"/foo/123*{args}", "/foo/123*{args}/bar", "/foo/123{ps}/bar"}, - path: "/foo/123a/bar", - wantPath: "/foo/123{ps}/bar", - wantTsr: false, - wantParams: Params{ - { - Key: "ps", - Value: "a", - }, - }, - }, - { - name: "overlapping infix inflight suffix wildcard and param match suffix", - routes: []string{"/foo/123*{args}", "/foo/123*{args}/bar", "/foo/123{ps}"}, - path: "/foo/123a/bili", - wantPath: "/foo/123*{args}", - wantTsr: false, - wantParams: Params{ - { - Key: "args", - Value: "a/bili", - }, - }, - }, - { - name: "overlapping infix inflight suffix wildcard and param match param", - routes: []string{"/foo/123*{args}", "/foo/123*{args}/bar", "/foo/123{ps}"}, - path: "/foo/123a", - wantPath: "/foo/123{ps}", - wantTsr: false, - wantParams: Params{ - { - Key: "ps", - Value: "a", - }, - }, - }, - { - name: "infix segment followed by param", - routes: []string{"/foo/*{a}/{b}"}, - path: "/foo/a/b/c/d", - wantPath: "/foo/*{a}/{b}", - wantTsr: false, - wantParams: Params{ - { - Key: "a", - Value: "a/b/c", - }, - { - Key: "b", - Value: "d", - }, - }, - }, - { - name: "infix segment followed by two params", - routes: []string{"/foo/*{a}/{b}/{c}"}, - path: "/foo/a/b/c/d", - wantPath: "/foo/*{a}/{b}/{c}", - wantTsr: false, - wantParams: Params{ - { - Key: "a", - Value: "a/b", - }, - { - Key: "b", - Value: "c", - }, - { - Key: "c", - Value: "d", - }, - }, - }, - { - name: "infix segment followed by one param and one wildcard", - routes: []string{"/foo/*{a}/{b}/*{c}"}, - path: "/foo/a/b/c/d", - wantPath: "/foo/*{a}/{b}/*{c}", - wantTsr: false, - wantParams: Params{ - { - Key: "a", - Value: "a", - }, - { - Key: "b", - Value: "b", - }, - { - Key: "c", - Value: "c/d", - }, - }, - }, - { - name: "param followed by suffix wildcard", - routes: []string{"/foo/{a}/*{b}"}, - path: "/foo/a/b/c/d", - wantPath: "/foo/{a}/*{b}", - wantTsr: false, - wantParams: Params{ - { - Key: "a", - Value: "a", - }, - { - Key: "b", - Value: "b/c/d", - }, - }, - }, - { - name: "infix inflight segment followed by param", - routes: []string{"/foo/123*{a}/{b}"}, - path: "/foo/123a/b/c/d", - wantPath: "/foo/123*{a}/{b}", - wantTsr: false, - wantParams: Params{ - { - Key: "a", - Value: "a/b/c", - }, - { - Key: "b", - Value: "d", - }, - }, - }, - { - name: "inflight param followed by suffix wildcard", - routes: []string{"/foo/123{a}/*{b}"}, - path: "/foo/123a/b/c/d", - wantPath: "/foo/123{a}/*{b}", - wantTsr: false, - wantParams: Params{ - { - Key: "a", - Value: "a", - }, - { - Key: "b", - Value: "b/c/d", - }, - }, - }, - { - name: "multi infix segment simple", - routes: []string{"/foo/*{$1}/bar/*{$2}/baz"}, - path: "/foo/a/bar/b/c/d/baz", - wantPath: "/foo/*{$1}/bar/*{$2}/baz", - wantTsr: false, - wantParams: Params{ - { - Key: "$1", - Value: "a", - }, - { - Key: "$2", - Value: "b/c/d", - }, - }, - }, - { - name: "multi inflight segment simple", - routes: []string{"/foo/123*{$1}/bar/456*{$2}/baz"}, - path: "/foo/123a/bar/456b/c/d/baz", - wantPath: "/foo/123*{$1}/bar/456*{$2}/baz", - wantTsr: false, - wantParams: Params{ - { - Key: "$1", - Value: "a", - }, - { - Key: "$2", - Value: "b/c/d", - }, - }, - }, - { - name: "static priority", - routes: []string{"/foo/bar/baz", "/foo/{ps}/baz", "/foo/*{any}/baz"}, - path: "/foo/bar/baz", - wantPath: "/foo/bar/baz", - wantTsr: false, - }, - { - name: "param priority", - routes: []string{"/foo/bar/baz", "/foo/{ps}/baz", "/foo/*{any}/baz"}, - path: "/foo/buzz/baz", - wantPath: "/foo/{ps}/baz", - wantTsr: false, - wantParams: Params{ - { - Key: "ps", - Value: "buzz", - }, - }, - }, - { - name: "fallback catch all", - routes: []string{"/foo/bar/baz", "/foo/{ps}/baz", "/foo/*{any}/baz"}, - path: "/foo/a/b/baz", - wantPath: "/foo/*{any}/baz", - wantTsr: false, - wantParams: Params{ - { - Key: "any", - Value: "a/b", - }, - }, - }, - { - name: "complex overlapping route with static priority", - routes: []string{ - "/foo/bar/baz/{$1}/jo", - "/foo/*{any}/baz/{$1}/jo", - "/foo/{ps}/baz/{$1}/jo", - }, - path: "/foo/bar/baz/1/jo", - wantPath: "/foo/bar/baz/{$1}/jo", - wantTsr: false, - wantParams: Params{ - { - Key: "$1", - Value: "1", - }, - }, - }, - { - name: "complex overlapping route with param priority", - routes: []string{ - "/foo/bar/baz/{$1}/jo", - "/foo/*{any}/baz/{$1}/jo", - "/foo/{ps}/baz/{$1}/jo", - }, - path: "/foo/bam/baz/1/jo", - wantPath: "/foo/{ps}/baz/{$1}/jo", - wantTsr: false, - wantParams: Params{ - { - Key: "ps", - Value: "bam", - }, - { - Key: "$1", - Value: "1", - }, - }, - }, - { - name: "complex overlapping route with catch all fallback", - routes: []string{ - "/foo/bar/baz/{$1}/jo", - "/foo/*{any}/baz/{$1}/jo", - "/foo/{ps}/baz/{$1}/jo", - }, - path: "/foo/a/b/c/baz/1/jo", - wantPath: "/foo/*{any}/baz/{$1}/jo", - wantTsr: false, - wantParams: Params{ - { - Key: "any", - Value: "a/b/c", - }, - { - Key: "$1", - Value: "1", - }, - }, - }, - { - name: "complex overlapping route with catch all fallback", - routes: []string{ - "/foo/bar/baz/{$1}/jo", - "/foo/*{any}/baz/{$1}/john", - "/foo/{ps}/baz/{$1}/johnny", - }, - path: "/foo/a/baz/1/john", - wantPath: "/foo/*{any}/baz/{$1}/john", - wantTsr: false, - wantParams: Params{ - { - Key: "any", - Value: "a", - }, - { - Key: "$1", - Value: "1", - }, - }, - }, - { - name: "overlapping static and infix", - routes: []string{ - "/foo/*{any}/baz", - "/foo/a/b/baz", - }, - path: "/foo/a/b/baz", - wantPath: "/foo/a/b/baz", - wantTsr: false, - }, - { - name: "overlapping static and infix with catch all fallback", - routes: []string{ - "/foo/*{any}/baz", - "/foo/a/b/baz", - }, - path: "/foo/a/b/c/baz", - wantPath: "/foo/*{any}/baz", - wantTsr: false, - wantParams: Params{ - { - Key: "any", - Value: "a/b/c", - }, - }, - }, - { - name: "infix wildcard with trailing slash", - routes: []string{ - "/foo/*{any}/", - }, - path: "/foo/a/b/c/", - wantPath: "/foo/*{any}/", - wantTsr: false, - wantParams: Params{ - { - Key: "any", - Value: "a/b/c", - }, - }, - }, - { - name: "overlapping static and infix with most specific", - routes: []string{ - "/foo/*{any}/{a}/ddd/", - "/foo/*{any}/bbb/{d}", - }, - path: "/foo/a/b/c/bbb/ddd/", - wantPath: "/foo/*{any}/{a}/ddd/", - wantTsr: false, - wantParams: Params{ - { - Key: "any", - Value: "a/b/c", - }, - { - Key: "a", - Value: "bbb", - }, - }, - }, - { - name: "infix wildcard with trailing slash", - routes: []string{ - "/foo/*{any}", - "/foo/*{any}/b/", - "/foo/*{any}/c/", - }, - path: "/foo/x/y/z/", - wantPath: "/foo/*{any}", - wantTsr: false, - wantParams: Params{ - { - Key: "any", - Value: "x/y/z/", - }, - }, - }, - { - name: "infix wildcard with trailing slash most specific", - routes: []string{ - "/foo/*{any}", - "/foo/*{any}/", - }, - path: "/foo/x/y/z/", - wantPath: "/foo/*{any}/", - wantTsr: false, - wantParams: Params{ - { - Key: "any", - Value: "x/y/z", - }, - }, - }, - { - name: "infix wildcard with trailing slash most specific", - routes: []string{ - "/foo/*{any}", - "/foo/*{any}/", - }, - path: "/foo/x/y/z", - wantPath: "/foo/*{any}", - wantTsr: false, - wantParams: Params{ - { - Key: "any", - Value: "x/y/z", - }, - }, - }, +func TestTree_RaceDetector(t *testing.T) { + var wg sync.WaitGroup + start, wait := atomicSync() + var raceCount atomic.Uint32 + + tree := New().Tree() + + wg.Add(len(staticRoutes) * 3) + for _, rte := range staticRoutes { + go func() { + wait() + defer func() { + if v := recover(); v != nil { + raceCount.Add(1) + assert.ErrorIs(t, v.(error), ErrConcurrentAccess) + } + wg.Done() + }() + _ = tree.insert(rte.method, &Route{path: rte.path}, 0) + }() + + go func() { + wait() + defer func() { + if v := recover(); v != nil { + raceCount.Add(1) + assert.ErrorIs(t, v.(error), ErrConcurrentAccess) + } + wg.Done() + }() + _ = tree.update(rte.method, &Route{path: rte.path}) + }() + + go func() { + wait() + defer func() { + if v := recover(); v != nil { + raceCount.Add(1) + assert.ErrorIs(t, v.(error), ErrConcurrentAccess) + } + wg.Done() + }() + tree.remove(rte.method, rte.path) + }() } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - f := New() - tree := f.Tree() - for _, rte := range tc.routes { - require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) - } - nds := *tree.nodes.Load() - c := newTestContextTree(tree) - n, tsr := tree.lookup(nds[0].children[0].Load(), tc.path, c, false) - require.NotNil(t, n) - assert.Equal(t, tc.wantPath, n.route.path) - assert.Equal(t, tc.wantTsr, tsr) - c.tsr = tsr - assert.Equal(t, tc.wantParams, slices.Collect(c.Params())) + time.Sleep(500 * time.Millisecond) + start() + wg.Wait() + assert.GreaterOrEqual(t, raceCount.Load(), uint32(1)) +} - fmt.Println(n) - }) - } +func TestConcurrentRequestHandling(t *testing.T) { + r := New() -} + // /repos/{owner}/{repo}/keys + h1 := HandlerFunc(func(c Context) { + assert.Equal(t, "john", c.Param("owner")) + assert.Equal(t, "fox", c.Param("repo")) + _ = c.String(200, c.Path()) + }) -func TestInfixWildcardTsr(t *testing.T) { - cases := []struct { - name string - routes []string - path string - wantPath string - wantTsr bool - wantParams []Param - }{ - { - name: "infix wildcard with trailing slash and tsr add", - routes: []string{ - "/foo/*{any}/", - }, - path: "/foo/a/b/c", - wantPath: "/foo/*{any}/", - wantTsr: true, - wantParams: Params{ - { - Key: "any", - Value: "a/b/c", - }, - }, - }, - { - name: "infix wildcard with tsr but skipped node match", - routes: []string{ - "/foo/*{any}/", - "/{x}/a/b/c", - }, - path: "/foo/a/b/c", - wantPath: "/{x}/a/b/c", - wantTsr: false, - wantParams: Params{ - { - Key: "x", - Value: "foo", - }, - }, - }, - { - name: "infix wildcard with tsr but skipped node does not match", - routes: []string{ - "/foo/*{any}/", - "/{x}/a/b/x", - }, - path: "/foo/a/b/c", - wantPath: "/foo/*{any}/", - wantTsr: true, - wantParams: Params{ - { - Key: "any", - Value: "a/b/c", - }, - }, - }, - { - name: "infix wildcard with trailing slash and tsr add", - routes: []string{ - "/foo/*{any}/", - "/foo/*{any}/abc", - "/foo/*{any}/bcd", - }, - path: "/foo/a/b/c/abd", - wantPath: "/foo/*{any}/", - wantTsr: true, - wantParams: Params{ - { - Key: "any", - Value: "a/b/c/abd", - }, - }, - }, - { - name: "infix wildcard with sub-node tsr add fallback", - routes: []string{ - "/foo/*{any}/{a}/ddd/", - "/foo/*{any}/bbb/{d}/foo", - }, - path: "/foo/a/b/c/bbb/ddd", - wantPath: "/foo/*{any}/{a}/ddd/", - wantTsr: true, - wantParams: Params{ - { - Key: "any", - Value: "a/b/c", - }, - { - Key: "a", - Value: "bbb", - }, - }, - }, - { - name: "infix wildcard with sub-node tsr at depth 1 but direct match", - routes: []string{ - "/foo/*{any}/c/bbb/", - "/foo/*{any}/bbb", - }, - path: "/foo/a/b/c/bbb", - wantPath: "/foo/*{any}/bbb", - wantTsr: false, - wantParams: Params{ - { - Key: "any", - Value: "a/b/c", - }, - }, - }, - { - name: "infix wildcard with sub-node tsr at depth 1 and 2 but direct match", - routes: []string{ - "/foo/*{any}/b/c/bbb/", - "/foo/*{any}/c/bbb/", - "/foo/*{any}/bbb", - }, - path: "/foo/a/b/c/bbb", - wantPath: "/foo/*{any}/bbb", - wantTsr: false, - wantParams: Params{ - { - Key: "any", - Value: "a/b/c", - }, - }, - }, - { - name: "infix wildcard with sub-node tsr at depth 1 and 2 but fallback first tsr", - routes: []string{ - "/foo/*{any}/b/c/bbb/", - "/foo/*{any}/c/bbb/", - "/foo/*{any}/bbx", - }, - path: "/foo/a/b/c/bbb", - wantPath: "/foo/*{any}/b/c/bbb/", - wantTsr: true, - wantParams: Params{ - { - Key: "any", - Value: "a", - }, - }, - }, - { - name: "infix wildcard with sub-node tsr at depth 1 and 2 but fallback first tsr", - routes: []string{ - "/foo/*{any}/", - "/foo/*{any}/b/c/bbb/", - "/foo/*{any}/c/bbb/", - "/foo/*{any}/bbx", - }, - path: "/foo/a/b/c/bbb", - wantPath: "/foo/*{any}/b/c/bbb/", - wantTsr: true, - wantParams: Params{ - { - Key: "any", - Value: "a", - }, - }, - }, - { - name: "infix wildcard with depth 0 tsr and sub-node tsr at depth 1 fallback first tsr", - routes: []string{ - "/foo/a/b/c/bbb/", - "/foo/*{any}/c/bbb/", - "/foo/*{any}/bbx", - }, - path: "/foo/a/b/c/bbb", - wantPath: "/foo/a/b/c/bbb/", - wantTsr: true, - }, - { - name: "infix wildcard with depth 0 tsr and sub-node tsr at depth 1 fallback first tsr", - routes: []string{ - "/foo/{first}/b/c/bbb/", - "/foo/*{any}/c/bbb/", - "/foo/*{any}/bbx", - }, - path: "/foo/a/b/c/bbb", - wantPath: "/foo/{first}/b/c/bbb/", - wantTsr: true, - wantParams: Params{ - { - Key: "first", - Value: "a", - }, - }, - }, - { - name: "multi infix wildcard with sub-node tsr at depth 1 but direct match", - routes: []string{ - "/foo/*{any1}/b/c/*{any2}/d/", - "/foo/*{any1}/c/*{any2}/d", - }, - path: "/foo/a/b/c/x/y/z/d", - wantPath: "/foo/*{any1}/c/*{any2}/d", - wantTsr: false, - wantParams: Params{ - { - Key: "any1", - Value: "a/b", - }, - { - Key: "any2", - Value: "x/y/z", - }, - }, - }, - { - name: "multi infix wildcard with sub-node tsr at depth 1 and fallback first", - routes: []string{ - "/foo/*{any1}/b/c/*{any2}/d/", - "/foo/*{any1}/c/*{any2}/ff", - }, - path: "/foo/a/b/c/x/y/z/d", - wantPath: "/foo/*{any1}/b/c/*{any2}/d/", - wantTsr: true, - wantParams: Params{ - { - Key: "any1", - Value: "a", - }, - { - Key: "any2", - Value: "x/y/z", - }, - }, - }, + // /repos/{owner}/{repo}/contents/*{path} + h2 := HandlerFunc(func(c Context) { + assert.Equal(t, "alex", c.Param("owner")) + assert.Equal(t, "vault", c.Param("repo")) + assert.Equal(t, "file.txt", c.Param("path")) + _ = c.String(200, c.Path()) + }) + + // /users/{user}/received_events/public + h3 := HandlerFunc(func(c Context) { + assert.Equal(t, "go", c.Param("user")) + _ = c.String(200, c.Path()) + }) + + require.NoError(t, onlyError(r.Handle(http.MethodGet, "/repos/{owner}/{repo}/keys", h1))) + require.NoError(t, onlyError(r.Handle(http.MethodGet, "/repos/{owner}/{repo}/contents/*{path}", h2))) + require.NoError(t, onlyError(r.Handle(http.MethodGet, "/users/{user}/received_events/public", h3))) + + r1 := httptest.NewRequest(http.MethodGet, "/repos/john/fox/keys", nil) + r2 := httptest.NewRequest(http.MethodGet, "/repos/alex/vault/contents/file.txt", nil) + r3 := httptest.NewRequest(http.MethodGet, "/users/go/received_events/public", nil) + + var wg sync.WaitGroup + wg.Add(300) + start, wait := atomicSync() + for i := 0; i < 100; i++ { + go func() { + defer wg.Done() + wait() + w := httptest.NewRecorder() + r.ServeHTTP(w, r1) + assert.Equal(t, "/repos/{owner}/{repo}/keys", w.Body.String()) + }() + + go func() { + defer wg.Done() + wait() + w := httptest.NewRecorder() + r.ServeHTTP(w, r2) + assert.Equal(t, "/repos/{owner}/{repo}/contents/*{path}", w.Body.String()) + }() + + go func() { + defer wg.Done() + wait() + w := httptest.NewRecorder() + r.ServeHTTP(w, r3) + assert.Equal(t, "/users/{user}/received_events/public", w.Body.String()) + }() } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - f := New() - tree := f.Tree() - for _, rte := range tc.routes { - require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) - } - nds := *tree.nodes.Load() - c := newTestContextTree(tree) - n, tsr := tree.lookup(nds[0].children[0].Load(), tc.path, c, false) - require.NotNil(t, n) - assert.Equal(t, tc.wantPath, n.route.path) - assert.Equal(t, tc.wantTsr, tsr) - c.tsr = tsr - assert.Equal(t, tc.wantParams, slices.Collect(c.Params())) + start() + wg.Wait() +} - fmt.Println(n) - fmt.Println("nodes:", nds[0]) - }) +func atomicSync() (start func(), wait func()) { + var n int32 + + start = func() { + atomic.StoreInt32(&n, 1) + } + + wait = func() { + for atomic.LoadInt32(&n) != 1 { + time.Sleep(1 * time.Microsecond) + } + } + + return +} + +func TestNode_String(t *testing.T) { + f := New() + require.NoError(t, onlyError(f.Handle(http.MethodGet, "/foo/{bar}/*{baz}", emptyHandler))) + tree := f.Tree() + nds := *tree.nodes.Load() + + want := `path: GET + path: /foo/{bar}/*{baz} [leaf=/foo/{bar}/*{baz}] [bar (10), baz (-1)]` + assert.Equal(t, want, strings.TrimSuffix(nds[0].String(), "\n")) +} + +func TestFixTrailingSlash(t *testing.T) { + assert.Equal(t, "/foo/", FixTrailingSlash("/foo")) + assert.Equal(t, "/foo", FixTrailingSlash("/foo/")) + assert.Equal(t, "/", FixTrailingSlash("")) +} + +// This example demonstrates how to create a simple router using the default options, +// which include the Recovery and Logger middleware. A basic route is defined, along with a +// custom middleware to log the request metrics. +func ExampleNew() { + // Create a new router with default options, which include the Recovery and Logger middleware + r := New(DefaultOptions()) + + // Define a custom middleware to measure the time taken for request processing and + // log the URL, route, time elapsed, and status code. + metrics := func(next HandlerFunc) HandlerFunc { + return func(c Context) { + start := time.Now() + next(c) + log.Printf("url=%s; route=%s; time=%d; status=%d", c.Request().URL, c.Path(), time.Since(start), c.Writer().Status()) + } + } + + // Define a route with the path "/hello/{name}", apply the custom "metrics" middleware, + // and set a simple handler that greets the user by their name. + r.MustHandle(http.MethodGet, "/hello/{name}", metrics(func(c Context) { + _ = c.String(200, "Hello %s\n", c.Param("name")) + })) + + // Start the HTTP server using the router as the handler and listen on port 8080 + log.Fatalln(http.ListenAndServe(":8080", r)) +} + +// This example demonstrates how to register a global middleware that will be +// applied to all routes. +func ExampleWithMiddleware() { + // Define a custom middleware to measure the time taken for request processing and + // log the URL, route, time elapsed, and status code. + metrics := func(next HandlerFunc) HandlerFunc { + return func(c Context) { + start := time.Now() + next(c) + log.Printf( + "url=%s; route=%s; time=%d; status=%d", + c.Request().URL, + c.Path(), + time.Since(start), + c.Writer().Status(), + ) + } + } + + f := New(WithMiddleware(metrics)) + + f.MustHandle(http.MethodGet, "/hello/{name}", func(c Context) { + _ = c.String(200, "Hello %s\n", c.Param("name")) + }) +} + +// This example demonstrates some important considerations when using the Tree API. +func ExampleRouter_Tree() { + r := New() + + // Each tree as its own sync.Mutex that is used to lock write on the tree. Since the router tree may be swapped at + // any given time, you MUST always copy the pointer locally, This ensures that you do not inadvertently cause a + // deadlock by locking/unlocking the wrong tree. + tree := r.Tree() + upsert := func(method, path string, handler HandlerFunc) (*Route, error) { + tree.Lock() + defer tree.Unlock() + if tree.Has(method, path) { + return tree.Update(method, path, handler) + } + return tree.Handle(method, path, handler) + } + + _, _ = upsert(http.MethodGet, "/foo/bar", func(c Context) { + // Note the tree accessible from fox.Context is already a local copy so the golden rule above does not apply. + c.Tree().Lock() + defer c.Tree().Unlock() + _ = c.String(200, "foo bar") + }) + + // Bad, instead make a local copy of the tree! + _ = func(method, path string, handler HandlerFunc) (*Route, error) { + r.Tree().Lock() + defer r.Tree().Unlock() + if r.Tree().Has(method, path) { + return r.Tree().Update(method, path, handler) + } + return r.Tree().Handle(method, path, handler) } +} + +// This example demonstrates how to create a custom middleware that cleans the request path and performs a manual +// lookup on the tree. If the cleaned path matches a registered route, the client is redirected to the valid path. +func ExampleRouter_Lookup() { + redirectFixedPath := MiddlewareFunc(func(next HandlerFunc) HandlerFunc { + return func(c Context) { + req := c.Request() + target := req.URL.Path + cleanedPath := CleanPath(target) + + // Nothing to clean, call next handler. + if cleanedPath == target { + next(c) + return + } + + req.URL.Path = cleanedPath + route, cc, tsr := c.Fox().Lookup(c.Writer(), req) + if route != nil { + defer cc.Close() + + code := http.StatusMovedPermanently + if req.Method != http.MethodGet { + code = http.StatusPermanentRedirect + } + + // Redirect the client if direct match or indirect match. + if !tsr || route.IgnoreTrailingSlashEnabled() { + if err := c.Redirect(code, cleanedPath); err != nil { + // Only if not in the range 300..308, so not possible here! + panic(err) + } + return + } + + // Add or remove an extra trailing slash and redirect the client. + if route.RedirectTrailingSlashEnabled() { + if err := c.Redirect(code, FixTrailingSlash(cleanedPath)); err != nil { + // Only if not in the range 300..308, so not possible here + panic(err) + } + return + } + } + + // rollback to the original path before calling the + // next handler or middleware. + req.URL.Path = target + next(c) + } + }) + f := New( + // Register the middleware for the NoRouteHandler scope. + WithMiddlewareFor(NoRouteHandler|NoMethodHandler, redirectFixedPath), + ) + + f.MustHandle(http.MethodGet, "/hello/{name}", func(c Context) { + _ = c.String(200, "Hello %s\n", c.Param("name")) + }) +} + +// This example demonstrates how to do a reverse lookup on the tree. +func ExampleTree_Reverse() { + f := New() + f.MustHandle(http.MethodGet, "/hello/{name}", emptyHandler) + + tree := f.Tree() + route, _ := tree.Reverse(http.MethodGet, "/hello/fox") + fmt.Println(route.Path()) // /hello/{name} +} + +// This example demonstrates how to check if a given route is registered in the tree. +func ExampleTree_Has() { + f := New() + f.MustHandle(http.MethodGet, "/hello/{name}", emptyHandler) + + tree := f.Tree() + exist := tree.Has(http.MethodGet, "/hello/{name}") + fmt.Println(exist) // true +} + +func onlyError[T any](_ T, err error) error { + return err } diff --git a/tree.go b/tree.go index 873eb0f..70ef4bf 100644 --- a/tree.go +++ b/tree.go @@ -8,7 +8,6 @@ import ( "fmt" "net/http" "slices" - "strconv" "strings" "sync" "sync/atomic" @@ -32,7 +31,6 @@ import ( // defer fox.Tree().Unlock() type Tree struct { ctx sync.Pool - np sync.Pool nodes atomic.Pointer[[]*node] fox *Router sync.Mutex @@ -554,11 +552,6 @@ Walk: break } - x := string(current.key[i]) - y := string(path[charsMatched]) - _ = x - _ = y - if current.key[i] != path[charsMatched] || path[charsMatched] == bracketDelim || path[charsMatched] == '*' { if current.key[i] == '{' { startPath := charsMatched @@ -600,14 +593,19 @@ Walk: idx := current.params[paramKeyCnt].end - charsMatchedInNodeFound var interNode *node if idx >= 0 { - interNode = t.np.Get().(*node) - interNode.params = interNode.params[:0] - interNode.route = current.route - interNode.key = current.key[current.params[paramKeyCnt].end:] - interNode.childKeys = current.childKeys - interNode.children = current.children - interNode.paramChildIndex = current.paramChildIndex - interNode.wildcardChildIndex = current.wildcardChildIndex + // Unfortunately, we cannot use object pooling here because we need to keep a reference to this + // interNode object until the lookup function returns, especially when TSR (Trailing Slash Redirect) + // is enabled. The interNode may be referenced by subNode and 'n'. + interNode = &node{ + route: current.route, + key: current.key[current.params[paramKeyCnt].end:], + childKeys: current.childKeys, + children: current.children, + // len(current.params)-1 is safe since we have at least the current infix wildcard in params + params: make([]param, 0, len(current.params)-1), + paramChildIndex: current.paramChildIndex, + wildcardChildIndex: current.wildcardChildIndex, + } for _, ps := range current.params[paramKeyCnt+1:] { // paramKeyCnt+1 is safe since we have at least the current infix wildcard in params interNode.params = append(interNode.params, param{ key: ps.key, @@ -633,21 +631,13 @@ Walk: subCtx := t.ctx.Get().(*cTx) startPath := charsMatched - y := path[charsMatched:] - _ = y for { idx := strings.IndexByte(path[charsMatched:], slashDelim) // idx >= 0, we have a next segment with at least one char if idx > 0 { *subCtx.params = (*subCtx.params)[:0] charsMatched += idx - x := path[charsMatched:] - _ = x - a := strconv.Itoa(depth) - _ = a - depth++ subNode, subTsr := t.lookup(interNode, path[charsMatched:], subCtx, false) - depth-- if subNode == nil { // Try with next segment charsMatched++ @@ -659,15 +649,6 @@ Walk: // Only if no previous tsr if !tsr { tsr = true - /* nodeCopy := &node{ - route: subNode.route, - key: subNode.key, - childKeys: subNode.childKeys, - children: subNode.children, - params: subNode.params, - paramChildIndex: subNode.paramChildIndex, - wildcardChildIndex: subNode.wildcardChildIndex, - }*/ n = subNode if !lazy { *c.tsrParams = (*c.tsrParams)[:0] @@ -687,13 +668,11 @@ Walk: *c.params = append(*c.params, *subCtx.params...) } - t.np.Put(interNode) t.ctx.Put(subCtx) return subNode, subTsr } t.ctx.Put(subCtx) - // t.np.Put(interNode) // We can record params here because it may be either an ending catch-all node (leaf=/foo/*{args}) with // children, or we may have a tsr opportunity (leaf=/foo/*{args}/ with /foo/x/y/z path). Note that if @@ -777,9 +756,6 @@ Walk: } } - a := strconv.Itoa(depth) - _ = a - paramCnt = 0 paramKeyCnt = 0 hasSkpNds := len(*c.skipNds) > 0 @@ -985,14 +961,6 @@ func (t *Tree) allocateContext() *cTx { } } -// allocateNode is only used on the lookup phase when we have to dynamically create an intermediary node -// to recursively call Tree.lookup with a subtree. -func (t *Tree) allocateNode() *node { - return &node{ - params: make([]param, 0, t.maxParams.Load()), - } -} - // addRoot append a new root node to the tree. // Note: This function should be guarded by mutex. func (t *Tree) addRoot(n *node) { From 32c5ba2f51af3d13cd14c414bcdd3decc487aa23 Mon Sep 17 00:00:00 2001 From: tigerwill90 Date: Thu, 24 Oct 2024 21:11:27 +0200 Subject: [PATCH 09/15] feat: improve test coverage --- fox_test.go | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/fox_test.go b/fox_test.go index b387dd8..33346a1 100644 --- a/fox_test.go +++ b/fox_test.go @@ -1420,6 +1420,32 @@ func TestInfixWildcard(t *testing.T) { }, }, }, + { + name: "simple infix wildcard", + routes: []string{"/foo/*{args}/bar"}, + path: "/foo/a/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a", + }, + }, + }, + { + name: "static with infix wildcard child", + routes: []string{"/foo/", "/foo/*{args}/baz"}, + path: "/foo/bar/baz", + wantPath: "/foo/*{args}/baz", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "bar", + }, + }, + }, { name: "simple infix wildcard with route char", routes: []string{"/foo/*{args}/bar"}, @@ -2103,8 +2129,6 @@ func TestInfixWildcard(t *testing.T) { assert.Equal(t, tc.wantTsr, tsr) c.tsr = tsr assert.Equal(t, tc.wantParams, slices.Collect(c.Params())) - - fmt.Println(n) }) } From f6599393f5a29b62b782732f451524240bf88db9 Mon Sep 17 00:00:00 2001 From: tigerwill90 Date: Thu, 24 Oct 2024 21:15:13 +0200 Subject: [PATCH 10/15] feat: improve test coverage --- fox_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/fox_test.go b/fox_test.go index 33346a1..62b2f64 100644 --- a/fox_test.go +++ b/fox_test.go @@ -2802,6 +2802,18 @@ func TestParseRoute(t *testing.T) { wantErr: ErrInvalidRoute, wantN: 0, }, + { + name: "empty infix catch all", + path: "/foo/bar/*{}/baz", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "empty ending catch all", + path: "/foo/bar/baz/*{}", + wantErr: ErrInvalidRoute, + wantN: 0, + }, { name: "unexpected character in param", path: "/foo/{{bar}", From 33d5154b966ed9967c20ea9e97618c76a94f9f01 Mon Sep 17 00:00:00 2001 From: tigerwill90 Date: Thu, 24 Oct 2024 21:31:51 +0200 Subject: [PATCH 11/15] feat: fix lint --- tree.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tree.go b/tree.go index 70ef4bf..4a035dd 100644 --- a/tree.go +++ b/tree.go @@ -530,8 +530,6 @@ const ( bracketDelim = '{' ) -var depth int - func (t *Tree) lookup(target *node, path string, c *cTx, lazy bool) (n *node, tsr bool) { var ( charsMatched int @@ -679,7 +677,6 @@ Walk: // there is no tsr opportunity, and skipped nodes > 0, we will truncate the params anyway. if !lazy { *c.params = append(*c.params, Param{Key: current.params[paramKeyCnt].key, Value: path[startPath:]}) - paramCnt++ } // We are also in an ending catch all, and this is the most specific path @@ -688,9 +685,6 @@ Walk: } charsMatched += len(path[charsMatched:]) - // and this point len(path) == charsMatched - ctrl := len(path) == charsMatched - _ = ctrl break Walk } From 17c29a6131f64ec330eea18e015612e52dcac314 Mon Sep 17 00:00:00 2001 From: tigerwill90 Date: Thu, 24 Oct 2024 21:50:04 +0200 Subject: [PATCH 12/15] feat: use const for star delim --- tree.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tree.go b/tree.go index 4a035dd..d04b5a8 100644 --- a/tree.go +++ b/tree.go @@ -528,6 +528,7 @@ func (t *Tree) remove(method, path string) bool { const ( slashDelim = '/' bracketDelim = '{' + starDelim = '*' ) func (t *Tree) lookup(target *node, path string, c *cTx, lazy bool) (n *node, tsr bool) { @@ -550,8 +551,8 @@ Walk: break } - if current.key[i] != path[charsMatched] || path[charsMatched] == bracketDelim || path[charsMatched] == '*' { - if current.key[i] == '{' { + if current.key[i] != path[charsMatched] || path[charsMatched] == bracketDelim || path[charsMatched] == starDelim { + if current.key[i] == bracketDelim { startPath := charsMatched idx := strings.IndexByte(path[charsMatched:], slashDelim) if idx > 0 { @@ -584,7 +585,7 @@ Walk: continue } - if current.key[i] == '*' { + if current.key[i] == starDelim { // | current.params[paramKeyCnt].end (10) // key: foo/*{bar}/ => 10 - 5 = 5 => i+=idx set i to '/' // | charsMatchedInNodeFound (5) From 7bf8ed3b5e790af68c9d0da3fbce651e3779fa89 Mon Sep 17 00:00:00 2001 From: tigerwill90 Date: Thu, 24 Oct 2024 22:25:10 +0200 Subject: [PATCH 13/15] feat: update README --- README.md | 68 ++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 47 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 9e531a3..3d63e6f 100644 --- a/README.md +++ b/README.md @@ -91,39 +91,57 @@ if errors.Is(err, fox.ErrRouteConflict) { ``` #### Named parameters -A route can be defined using placeholder (e.g `{name}`). The matching segment are recorder into `fox.Param` accessible -via `fox.Context`. `fox.Context.Params` provide an iterator to range over `fox.Param` and `fox.Context.Param` allow -to retrieve directly the value of a parameter using the placeholder name. +Routes can include named parameters using curly braces `{}` to match exactly one non-empty path segment. The matching +segment are recorder into `fox.Param` accessible via `fox.Context`. `fox.Context.Params` provide an iterator to range +over `fox.Param` and `fox.Context.Param` allow to retrieve directly the value of a parameter using the placeholder name. ```` Pattern /avengers/{name} -/avengers/ironman match -/avengers/thor match -/avengers/hulk/angry no match -/avengers/ no match +/avengers/ironman matches +/avengers/thor matches +/avengers/hulk/angry no matches +/avengers/ no matches Pattern /users/uuid:{id} -/users/uuid:123 match -/users/uuid no match +/users/uuid:123 matches +/users/uuid no matches ```` #### Catch all parameter -Catch-all parameters can be used to match everything at the end of a route. The placeholder start with `*` followed by a regular -named parameter (e.g. `*{name}`). +Catch-all parameters start with an asterisk `*` followed by a name `{param}` and match one or more non-empty path segments, +including slashes. They can be placed anywhere in the route path but **cannot be consecutive**. The matching segment are also +accessible via `fox.Context` + +**Example with ending catch all** ```` Pattern /src/*{filepath} -/src/ match -/src/conf.txt match -/src/dir/config.txt match +/src/conf.txt matches +/src/dir/config.txt matches +/src/ no matches + +Pattern /src/file=*{path} + +/src/file=config.txt matches +/src/file=/dir/config.txt matches +/src/file= no matches +```` + +**Example with infix catch all** +```` +Pattern: /assets/*{path}/thumbnail + +/assets/images/thumbnail matches +/assets/photos/2021/thumbnail matches +/assets/thumbnail no matches -Patter /src/file=*{path} +Pattern: /assets/path:*{path}/thumbnail -/src/file= match -/src/file=config.txt match -/src/file=/dir/config.txt match +/assets/path:images/thumbnail matches +/assets/path:photos/2021/thumbnail matches +/assets/path:thumbnail no matches ```` #### Priority rules @@ -151,9 +169,17 @@ POST /users/{name}/emails Additionally, let's consider an example to illustrate the prioritization: ```` -GET /fs/avengers.txt #1 => match /fs/avengers.txt -GET /fs/{filename} #2 => match /fs/ironman.txt -GET /fs/*{filepath} #3 => match /fs/avengers/ironman.txt +Route Definitions: + +1. GET /fs/avengers.txt # Highest priority (static) +2. GET /fs/{filename} # Next priority (named parameter) +3. GET /fs/*{filepath} # Lowest priority (catch-all parameter) + +Request Matching: + +- /fs/avengers.txt matches Route 1 +- /fs/ironman.txt matches Route 2 +- /fs/avengers/ironman.txt matches Route 3 ```` #### Warning about context From 482a0fc69834ddefd445cb9fb5ee99787de971e3 Mon Sep 17 00:00:00 2001 From: tigerwill90 Date: Thu, 24 Oct 2024 22:56:11 +0200 Subject: [PATCH 14/15] feat: update README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 3d63e6f..60c81af 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,7 @@ Pattern /users/uuid:{id} ```` #### Catch all parameter -Catch-all parameters start with an asterisk `*` followed by a name `{param}` and match one or more non-empty path segments, +Catch-all parameters start with an asterisk `*` followed by a name `{param}` and match one or more **non-empty** path segments, including slashes. They can be placed anywhere in the route path but **cannot be consecutive**. The matching segment are also accessible via `fox.Context` From e0a3c7cef53e91101abfc063a75a42324c76f9d3 Mon Sep 17 00:00:00 2001 From: tigerwill90 Date: Fri, 25 Oct 2024 00:17:34 +0200 Subject: [PATCH 15/15] feat: fix comment --- tree.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree.go b/tree.go index d04b5a8..da3dfdc 100644 --- a/tree.go +++ b/tree.go @@ -737,7 +737,7 @@ Walk: break } - // Here we have a next static segment and possibly wildcard children, so we save them for late evaluation if needed. + // Here we have a next static segment and possibly wildcard children, so we save them for later evaluation if needed. if current.wildcardChildIndex >= 0 { *c.skipNds = append(*c.skipNds, skippedNode{current, charsMatched, paramCnt, current.wildcardChildIndex}) }