From 7f4be762382f70edfc930e35ad349883eaf8d632 Mon Sep 17 00:00:00 2001 From: Sylvain Muller Date: Wed, 9 Oct 2024 18:55:44 +0200 Subject: [PATCH] [RFC] Rework on iterator using go 1.23 range over function (#38) * feat: wip per route options * feat: wip per route options * feat: wip per route options * Revert "feat: wip per route options" This reverts commit fb7f2e133292da1df0c37e2865e309ba4f7a234f. * feat: make sure that tree from different router cannot be swapped * feat: wip per route options * feat: wip per route options * feat: improve test coverage * feat: improve test coverage * feat: better handle stack frame skipping with recovery handler * feat: improve test coverage * feat: improve test coverage * feat: disable some lint rules * feat: replace sort.Slice by slices.SortFunc to improve sort performance * docs: fix comment for options that can be applied on a route basis. * feat: remove Route.HandleWithMiddleware API as it's not clear for now how and when to use it. * feat: make FixTrailingSlash public * feat: rework on the Match method * feat: rework on iterator using range over function * feat(iterator): optimize allocation on the stack when possible * feat(iter): improve test coverage * feat(iter): improve test coverage * feat(iter): use bigger stackSizeThreshold * feat(iter): improve test coverage --- fox.go | 38 +----- fox_test.go | 48 +++---- go.mod | 3 +- go.sum | 2 + iter.go | 362 ++++++++++++++++++++++++++------------------------- iter_test.go | 236 ++++++++++++++++++++++++++++----- tree.go | 54 ++------ 7 files changed, 430 insertions(+), 313 deletions(-) diff --git a/fox.go b/fox.go index 01f3bc0..f9617b1 100644 --- a/fox.go +++ b/fox.go @@ -5,7 +5,6 @@ package fox import ( - "errors" "fmt" "net" "net/http" @@ -283,39 +282,16 @@ func (fox *Router) Remove(method, path string) error { // be closed if non-nil. // This API is EXPERIMENTAL and is likely to change in future release. func (fox *Router) Lookup(w ResponseWriter, r *http.Request) (route *Route, cc ContextCloser, tsr bool) { - tree := fox.tree.Load() + tree := fox.Tree() return tree.Lookup(w, r) } -// SkipMethod is used as a return value from WalkFunc to indicate that -// the method named in the call is to be skipped. -var SkipMethod = errors.New("skip method") - -// WalkFunc is the type of the function called by Walk to visit each registered routes. -type WalkFunc func(method, path string, handler HandlerFunc) error - -// Walk allow to walk over all registered route in lexicographical order. If the function -// return the special value SkipMethod, Walk skips the current method. This function is -// safe for concurrent use by multiple goroutine and while mutation are ongoing. -// This API is EXPERIMENTAL and is likely to change in future release. -func Walk(tree *Tree, fn WalkFunc) error { - nds := *tree.nodes.Load() -Next: - for i := range nds { - method := nds[i].key - it := newRawIterator(nds[i]) - for it.hasNext() { - err := fn(method, it.path, it.current.route.handler) - if err != nil { - if errors.Is(err, SkipMethod) { - continue Next - } - return err - } - } - - } - return nil +// Iter returns an iterator that provides access to a collection of iterators for traversing the routing tree. +// This function is safe for concurrent use by multiple goroutines and can operate while the Tree is being modified. +// This API is EXPERIMENTAL and may change in future releases. +func (fox *Router) Iter() Iter { + tree := fox.Tree() + return tree.Iter() } // DefaultNotFoundHandler is a simple HandlerFunc that replies to each request diff --git a/fox_test.go b/fox_test.go index b79eb99..cfe2bdc 100644 --- a/fox_test.go +++ b/fox_test.go @@ -13,6 +13,7 @@ import ( "net/http/httptest" "reflect" "regexp" + "slices" "strings" "sync" "sync/atomic" @@ -2132,11 +2133,8 @@ func TestTree_Remove(t *testing.T) { require.NoError(t, tree.Remove(rte.method, rte.path)) } - cnt := 0 - _ = Walk(tree, func(method, path string, handler HandlerFunc) error { - cnt++ - return nil - }) + it := tree.Iter() + cnt := len(slices.Collect(right(it.All()))) assert.Equal(t, 0, cnt) assert.Equal(t, 4, len(*tree.nodes.Load())) @@ -2155,14 +2153,14 @@ func TestTree_Methods(t *testing.T) { require.NoError(t, f.Handle(rte.method, rte.path, emptyHandler)) } - methods := f.Tree().Methods("/gists/123/star") + methods := slices.Sorted(left(f.Iter().Reverse(f.Iter().Methods(), "/gists/123/star"))) assert.Equal(t, []string{"DELETE", "GET", "PUT"}, methods) - methods = f.Tree().Methods("*") + methods = slices.Sorted(f.Iter().Methods()) assert.Equal(t, []string{"DELETE", "GET", "POST", "PUT"}, methods) // Ignore trailing slash disable - methods = f.Tree().Methods("/gists/123/star/") + methods = slices.Sorted(left(f.Iter().Reverse(f.Iter().Methods(), "/gists/123/star/"))) assert.Empty(t, methods) } @@ -2173,13 +2171,13 @@ func TestTree_MethodsWithIgnoreTsEnable(t *testing.T) { require.NoError(t, f.Handle(method, "/john/doe/", emptyHandler)) } - methods := f.Tree().Methods("/foo/bar/") + methods := slices.Sorted(left(f.Iter().Reverse(f.Iter().Methods(), "/foo/bar/"))) assert.Equal(t, []string{"DELETE", "GET", "PUT"}, methods) - methods = f.Tree().Methods("/john/doe") + methods = slices.Sorted(left(f.Iter().Reverse(f.Iter().Methods(), "/john/doe"))) assert.Equal(t, []string{"DELETE", "GET", "PUT"}, methods) - methods = f.Tree().Methods("/foo/bar/baz") + methods = slices.Sorted(left(f.Iter().Reverse(f.Iter().Methods(), "/foo/bar/baz"))) assert.Empty(t, methods) } @@ -2393,7 +2391,7 @@ func TestRouterWithAutomaticOptions(t *testing.T) { t.Run(tc.name, func(t *testing.T) { for _, method := range tc.methods { require.NoError(t, f.Tree().Handle(method, tc.path, func(c Context) { - c.SetHeader("Allow", strings.Join(c.Tree().Methods(c.Request().URL.Path), ", ")) + c.SetHeader("Allow", strings.Join(slices.Sorted(left(c.Tree().Iter().Reverse(c.Tree().Iter().Methods(), c.Request().URL.Path))), ", ")) c.Writer().WriteHeader(http.StatusNoContent) })) } @@ -2469,7 +2467,7 @@ func TestRouterWithAutomaticOptionsAndIgnoreTsOptionEnable(t *testing.T) { t.Run(tc.name, func(t *testing.T) { for _, method := range tc.methods { require.NoError(t, f.Tree().Handle(method, tc.path, func(c Context) { - c.SetHeader("Allow", strings.Join(c.Tree().Methods(c.Request().URL.Path), ", ")) + c.SetHeader("Allow", strings.Join(slices.Sorted(left(c.Tree().Iter().Reverse(c.Tree().Iter().Methods(), c.Request().URL.Path))), ", ")) c.Writer().WriteHeader(http.StatusNoContent) })) } @@ -2514,7 +2512,7 @@ func TestRouterWithAutomaticOptionsAndIgnoreTsOptionDisable(t *testing.T) { t.Run(tc.name, func(t *testing.T) { for _, method := range tc.methods { require.NoError(t, f.Tree().Handle(method, tc.path, func(c Context) { - c.SetHeader("Allow", strings.Join(c.Tree().Methods(c.Request().URL.Path), ", ")) + c.SetHeader("Allow", strings.Join(slices.Sorted(left(c.Tree().Iter().Reverse(c.Tree().Iter().Methods(), c.Request().URL.Path))), ", ")) c.Writer().WriteHeader(http.StatusNoContent) })) } @@ -2814,7 +2812,7 @@ func TestTree_Match(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - route, tsr := r.Tree().Match(http.MethodGet, tc.path) + route, tsr := r.Tree().Reverse(http.MethodGet, tc.path) if tc.want != "" { require.NotNil(t, route) assert.Equal(t, tc.want, route.Path()) @@ -2885,7 +2883,7 @@ func TestTree_MatchWithIgnoreTrailingSlashEnable(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - route, tsr := r.Tree().Match(http.MethodGet, tc.path) + route, tsr := r.Tree().Reverse(http.MethodGet, tc.path) if tc.want != "" { require.NotNil(t, route) assert.Equal(t, tc.want, route.Path()) @@ -3006,11 +3004,8 @@ func TestFuzzInsertLookupUpdateAndDelete(t *testing.T) { require.NoError(t, err) } - countPath := 0 - require.NoError(t, Walk(tree, func(method, path string, handler HandlerFunc) error { - countPath++ - return nil - })) + it := tree.Iter() + countPath := len(slices.Collect(right(it.All()))) assert.Equal(t, len(routes), countPath) for rte := range routes { @@ -3031,11 +3026,8 @@ func TestFuzzInsertLookupUpdateAndDelete(t *testing.T) { require.True(t, deleted) } - countPath = 0 - require.NoError(t, Walk(tree, func(method, path string, handler HandlerFunc) error { - countPath++ - return nil - })) + it = tree.Iter() + countPath = len(slices.Collect(right(it.All()))) assert.Equal(t, 0, countPath) } @@ -3340,12 +3332,12 @@ func ExampleRouter_Lookup() { } // This example demonstrates how to do a reverse lookup on the tree. -func ExampleTree_Match() { +func ExampleTree_Reverse() { f := New() f.MustHandle(http.MethodGet, "/hello/{name}", emptyHandler) tree := f.Tree() - route, _ := tree.Match(http.MethodGet, "/hello/fox") + route, _ := tree.Reverse(http.MethodGet, "/hello/fox") fmt.Println(route.Path()) // /hello/{name} } diff --git a/go.mod b/go.mod index 1067303..bfe0147 100644 --- a/go.mod +++ b/go.mod @@ -1,10 +1,11 @@ module github.com/tigerwill90/fox -go 1.21 +go 1.23 require ( github.com/google/gofuzz v1.2.0 github.com/stretchr/testify v1.9.0 + golang.org/x/sys v0.26.0 ) require ( diff --git a/go.sum b/go.sum index 3034cdf..13c9a5b 100644 --- a/go.sum +++ b/go.sum @@ -19,6 +19,8 @@ github.com/rogpeppe/go-internal v1.8.0 h1:FCbCCtXNOY3UtUuHUYaghJg4y7Fd14rXifAYUA github.com/rogpeppe/go-internal v1.8.0/go.mod h1:WmiCO8CzOY8rg0OYDC4/i/2WRWAB6poM+XZ2dLUbcbE= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +golang.org/x/sys v0.26.0 h1:KHjCJyddX0LoSTb3J+vWpupP9p0oznkqVk/IfjymZbo= +golang.org/x/sys v0.26.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= diff --git a/iter.go b/iter.go index bfe092b..9d6eeba 100644 --- a/iter.go +++ b/iter.go @@ -5,228 +5,238 @@ package fox import ( - "sort" + "iter" ) -type Iterator struct { - tree *Tree - method string - current *node - stacks []stack - valid bool - started bool -} - -// NewIterator returns an Iterator that traverses all registered routes in lexicographic order. -// An Iterator is safe to use when the router is serving request, when routing updates are ongoing or -// in parallel with other Iterators. Note that changes that happen while iterating over routes may not be reflected -// by the Iterator. This api is EXPERIMENTAL and is likely to change in future release. -func NewIterator(t *Tree) *Iterator { - return &Iterator{ - tree: t, - } -} - -func (it *Iterator) methods() map[string]*node { - nds := *it.tree.nodes.Load() - m := make(map[string]*node, len(nds)) - for i := range nds { - if len(nds[i].children) > 0 { - m[nds[i].key] = nds[i] - } - } - return m -} - -// SeekPrefix reset the iterator cursor to the first route starting with key. -// It does not keep tracking of previous seek. -func (it *Iterator) SeekPrefix(key string) { - nds := it.methods() - keys := make([]string, 0, len(nds)) - for method, n := range nds { - result := it.tree.search(n, key) - if result.isExactMatch() || result.isKeyMidEdge() { - nds[method] = result.matched - keys = append(keys, method) - } - } - - sort.Sort(sort.Reverse(sort.StringSlice(keys))) - stacks := make([]stack, 0, len(keys)) - for _, key := range keys { - stacks = append(stacks, stack{ - edges: []*node{nds[key]}, - method: key, - }) - } - - it.stacks = stacks -} - -// SeekMethod reset the iterator cursor to the first route for the given method. -// It does not keep tracking of previous seek. -func (it *Iterator) SeekMethod(method string) { - nds := it.methods() - stacks := make([]stack, 0, 1) - n, ok := nds[method] - if ok { - stacks = append(stacks, stack{ - edges: []*node{n}, - method: method, - }) +func newRawIterator(n *node) *rawIterator { + return &rawIterator{ + stack: []stack{{edges: []*node{n}}}, } - - it.stacks = stacks } -// SeekMethodPrefix reset the iterator cursor to the first route starting with key for the given method. -// It does not keep tracking of previous seek. -func (it *Iterator) SeekMethodPrefix(method, key string) { - nds := it.methods() - stacks := make([]stack, 0, 1) - n, ok := nds[method] - if ok { - result := it.tree.search(n, key) - if result.isExactMatch() || result.isKeyMidEdge() { - stacks = append(stacks, stack{ - edges: []*node{result.matched}, - method: method, - }) - } - } - - it.stacks = stacks +type rawIterator struct { + current *node + path string + stack []stack } -// Rewind reset the iterator cursor all the way to zero-th position which is the first method and route. -// It does not keep track of whether the cursor started with SeekPrefix, SeekMethod or SeekMethodPrefix. -func (it *Iterator) Rewind() { - nds := it.methods() - methods := make([]string, 0, len(nds)) - for method := range nds { - methods = append(methods, method) - } - - sort.Sort(sort.Reverse(sort.StringSlice(methods))) +const stackSizeThreshold = 15 - stacks := make([]stack, 0, len(methods)) - for _, method := range methods { - stacks = append(stacks, stack{ - edges: []*node{nds[method]}, - method: method, - }) - } - - it.stacks = stacks -} - -// Valid returns false when iteration is done. -func (it *Iterator) Valid() bool { - if !it.started { - it.started = true - it.Next() - return it.valid - } - return it.valid +type stack struct { + edges []*node } -// Next advance the iterator to the next route. Always check it.Valid() after a it.Next(). -func (it *Iterator) Next() { - for len(it.stacks) > 0 { - n := len(it.stacks) - last := it.stacks[n-1] +func (it *rawIterator) hasNext() bool { + for len(it.stack) > 0 { + n := len(it.stack) + last := it.stack[n-1] elem := last.edges[0] if len(last.edges) > 1 { - it.stacks[n-1].edges = last.edges[1:] + it.stack[n-1].edges = last.edges[1:] } else { - it.stacks = it.stacks[:n-1] + it.stack = it.stack[:n-1] } if len(elem.children) > 0 { - it.stacks = append(it.stacks, stack{edges: elem.getEdgesShallowCopy()}) + it.stack = append(it.stack, stack{edges: elem.getEdgesShallowCopy()}) } it.current = elem - if last.method != "" { - it.method = last.method - } if it.current.isLeaf() { - it.valid = true - return + it.path = elem.route.Path() + return true } } it.current = nil - it.method = "" - it.valid = false - it.started = false -} - -// Path returns the registered path for the current route. -func (it *Iterator) Path() string { - if it.current != nil { - return it.current.route.path - } - return "" + it.path = "" + return false } -// Method returns the http method for the current route. -func (it *Iterator) Method() string { - return it.method +type Iter struct { + t *Tree } -// Handler return the registered handler for the current route. -func (it *Iterator) Handler() HandlerFunc { - if it.current != nil { - return it.current.route.handler +// Methods returns a range iterator over all HTTP methods registered in the routing tree. +// This function is safe for concurrent use by multiple goroutine and while mutation on Tree are ongoing. +// This API is EXPERIMENTAL and is likely to change in future release. +func (it Iter) Methods() iter.Seq[string] { + return func(yield func(string) bool) { + nds := *it.t.nodes.Load() + for i := range nds { + if len(nds[i].children) > 0 { + if !yield(nds[i].key) { + return + } + } + } } - return nil } -type stack struct { - method string - edges []*node +// Routes returns a range iterator over all registered routes in the routing tree that exactly match the provided path +// for the given HTTP methods. +// +// This method performs a lookup for each method and the exact route associated with the provided `path`. It yields +// a tuple containing the HTTP method and the corresponding route if the route is registered for that method and path. +// +// This function is safe for concurrent use by multiple goroutine and while mutation on Tree are ongoing. +// This API is EXPERIMENTAL and is likely to change in future release. +func (it Iter) Routes(methods iter.Seq[string], path string) iter.Seq2[string, *Route] { + return func(yield func(string, *Route) bool) { + nds := *it.t.nodes.Load() + c := it.t.ctx.Get().(*cTx) + defer c.Close() + for method := range methods { + c.resetNil() + index := findRootNode(method, nds) + if index < 0 || len(nds[index].children) == 0 { + continue + } + + n, tsr := it.t.lookup(nds[index], path, c, true) + if n != nil && !tsr && n.route.path == path { + if !yield(method, n.route) { + return + } + } + } + } } -func newRawIterator(n *node) *rawIterator { - return &rawIterator{ - stack: []stack{{edges: []*node{n}}}, +// Reverse returns a range iterator over all routes registered in the routing tree that match the given path +// for the provided HTTP methods. It performs a reverse lookup for each method and path combination, +// yielding a tuple containing the HTTP method and the corresponding route. +// Unlike Routes, which matches an exact route path, Reverse is used to match a path (e.g., a path from an incoming +// request) to registered routes in the tree. +// +// When WithIgnoreTrailingSlash or WithRedirectTrailingSlash is enabled, Reverse will match routes regardless +// of whether a trailing slash is present. +// +// This function is safe for concurrent use by multiple goroutine and while mutation on Tree are ongoing. +// This API is EXPERIMENTAL and may change in future releases. +func (it Iter) Reverse(methods iter.Seq[string], path string) iter.Seq2[string, *Route] { + return func(yield func(string, *Route) bool) { + nds := *it.t.nodes.Load() + c := it.t.ctx.Get().(*cTx) + defer c.Close() + for method := range methods { + c.resetNil() + index := findRootNode(method, nds) + if index < 0 || len(nds[index].children) == 0 { + continue + } + + n, tsr := it.t.lookup(nds[index], path, c, true) + if n != nil && (!tsr || n.route.redirectTrailingSlash || n.route.ignoreTrailingSlash) { + if !yield(method, n.route) { + return + } + } + } } } -type rawIterator struct { - current *node - path string - stack []stack -} +// Prefix returns a range iterator over all routes in the routing tree that match a given prefix and HTTP methods. +// +// This iterator traverses the routing tree for each method provided, starting from nodes that match the given prefix. +// For each method, it yields a tuple containing the HTTP method and the corresponding route found under that prefix. +// If no routes match the prefix, the method will not yield any results. +// +// This function is safe for concurrent use by multiple goroutine and while mutation on Tree are ongoing. +// This API is EXPERIMENTAL and may change in future releases. +func (it Iter) Prefix(methods iter.Seq[string], prefix string) iter.Seq2[string, *Route] { + return func(yield func(string, *Route) bool) { + nds := *it.t.nodes.Load() + maxDepth := it.t.maxDepth.Load() + var stacks []stack + if maxDepth < stackSizeThreshold { + stacks = make([]stack, 0, stackSizeThreshold) // stack allocation + } else { + stacks = make([]stack, 0, maxDepth) // heap allocation + } -func (it *rawIterator) hasNext() bool { - for len(it.stack) > 0 { - n := len(it.stack) - last := it.stack[n-1] - elem := last.edges[0] + for method := range methods { + index := findRootNode(method, nds) + if index < 0 || len(nds[index].children) == 0 { + continue + } - if len(last.edges) > 1 { - it.stack[n-1].edges = last.edges[1:] - } else { - it.stack = it.stack[:n-1] + result := it.t.search(nds[index], prefix) + if !result.isExactMatch() && !result.isKeyMidEdge() { + continue + } + + stacks = append(stacks, stack{ + edges: []*node{result.matched}, + }) + + for len(stacks) > 0 { + n := len(stacks) + last := stacks[n-1] + elem := last.edges[0] + + if len(last.edges) > 1 { + stacks[n-1].edges = last.edges[1:] + } else { + stacks = stacks[:n-1] + } + + if len(elem.children) > 0 { + stacks = append(stacks, stack{edges: elem.getEdgesShallowCopy()}) + } + + if elem.isLeaf() { + if !yield(method, elem.route) { + return + } + } + } } + } +} - if len(elem.children) > 0 { - it.stack = append(it.stack, stack{edges: elem.getEdgesShallowCopy()}) +// All returns a range iterator over all routes registered in the routing tree for all HTTP methods. +// The result is an iterator that yields a tuple containing the HTTP method and the corresponding route. +// This function is safe for concurrent use by multiple goroutine and while mutation on Tree are ongoing. +// This API is EXPERIMENTAL and may change in future releases. +func (it Iter) All() iter.Seq2[string, *Route] { + return func(yield func(string, *Route) bool) { + for method, route := range it.Prefix(it.Methods(), "/") { + if !yield(method, route) { + return + } } + } +} - it.current = elem +func left[K, V any](seq iter.Seq2[K, V]) iter.Seq[K] { + return func(yield func(K) bool) { + for k := range seq { + if !yield(k) { + return + } + } + } +} - if it.current.isLeaf() { - it.path = elem.route.Path() - return true +func right[K, V any](seq iter.Seq2[K, V]) iter.Seq[V] { + return func(yield func(V) bool) { + for _, v := range seq { + if !yield(v) { + return + } } } +} - it.current = nil - it.path = "" - return false +func seqOf[E any](elems ...E) iter.Seq[E] { + return func(yield func(E) bool) { + for _, e := range elems { + if !yield(e) { + return + } + } + } } diff --git a/iter_test.go b/iter_test.go index 8e79e59..aa046a1 100644 --- a/iter_test.go +++ b/iter_test.go @@ -9,12 +9,13 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "net/http" + "slices" "testing" ) var routesCases = []string{"/fox/router", "/foo/bar/{baz}", "/foo/bar/{baz}/{name}", "/john/doe/*{args}", "/john/doe"} -func TestIterator_Rewind(t *testing.T) { +func TestIter_Routes(t *testing.T) { tree := New().Tree() for _, rte := range routesCases { require.NoError(t, tree.Handle(http.MethodGet, rte, emptyHandler)) @@ -23,11 +24,32 @@ func TestIterator_Rewind(t *testing.T) { } results := make(map[string][]string) + it := tree.Iter() + for method, route := range it.Routes(it.Methods(), "/foo/bar/{baz}/{name}") { + assert.NotNil(t, route) + results[method] = append(results[method], route.Path()) + } - it := NewIterator(tree) - for it.Rewind(); it.Valid(); it.Next() { - assert.NotNil(t, it.Handler()) - results[it.Method()] = append(results[it.method], it.Path()) + want := []string{"/foo/bar/{baz}/{name}"} + for key := range results { + assert.ElementsMatch(t, want, results[key]) + } +} + +func TestIter_All(t *testing.T) { + tree := New().Tree() + for _, rte := range routesCases { + require.NoError(t, tree.Handle(http.MethodGet, rte, emptyHandler)) + require.NoError(t, tree.Handle(http.MethodPost, rte, emptyHandler)) + require.NoError(t, tree.Handle(http.MethodHead, rte, emptyHandler)) + } + + results := make(map[string][]string) + + it := tree.Iter() + for method, route := range it.All() { + assert.NotNil(t, route) + results[method] = append(results[method], route.Path()) } for key := range results { @@ -35,7 +57,73 @@ func TestIterator_Rewind(t *testing.T) { } } -func TestIterator_SeekMethod(t *testing.T) { +func TestIter_AllBreak(t *testing.T) { + tree := New().Tree() + for _, rte := range routesCases { + require.NoError(t, tree.Handle(http.MethodGet, rte, emptyHandler)) + require.NoError(t, tree.Handle(http.MethodPost, rte, emptyHandler)) + require.NoError(t, tree.Handle(http.MethodHead, rte, emptyHandler)) + } + + var ( + lastMethod string + lastRoute *Route + ) + it := tree.Iter() + for method, route := range it.All() { + lastMethod = method + lastRoute = route + break + } + assert.Equal(t, "GET", lastMethod) + assert.Equal(t, "/foo/bar/{baz}", lastRoute.Path()) +} + +func TestIter_ReverseBreak(t *testing.T) { + tree := New().Tree() + for _, rte := range routesCases { + require.NoError(t, tree.Handle(http.MethodGet, rte, emptyHandler)) + require.NoError(t, tree.Handle(http.MethodPost, rte, emptyHandler)) + require.NoError(t, tree.Handle(http.MethodHead, rte, emptyHandler)) + } + + var ( + lastMethod string + lastRoute *Route + ) + it := tree.Iter() + for method, route := range it.Reverse(it.Methods(), "/john/doe/1/2/3") { + lastMethod = method + lastRoute = route + break + } + assert.Equal(t, "GET", lastMethod) + assert.Equal(t, "/john/doe/*{args}", lastRoute.Path()) +} + +func TestIter_RouteBreak(t *testing.T) { + tree := New().Tree() + for _, rte := range routesCases { + require.NoError(t, tree.Handle(http.MethodGet, rte, emptyHandler)) + require.NoError(t, tree.Handle(http.MethodPost, rte, emptyHandler)) + require.NoError(t, tree.Handle(http.MethodHead, rte, emptyHandler)) + } + + var ( + lastMethod string + lastRoute *Route + ) + it := tree.Iter() + for method, route := range it.Routes(it.Methods(), "/john/doe/*{args}") { + lastMethod = method + lastRoute = route + break + } + assert.Equal(t, "GET", lastMethod) + assert.Equal(t, "/john/doe/*{args}", lastRoute.Path()) +} + +func TestIter_RootPrefixOneMethod(t *testing.T) { tree := New().Tree() for _, rte := range routesCases { require.NoError(t, tree.Handle(http.MethodGet, rte, emptyHandler)) @@ -44,18 +132,18 @@ func TestIterator_SeekMethod(t *testing.T) { } results := make(map[string][]string) + it := tree.Iter() - it := NewIterator(tree) - for it.SeekMethod(http.MethodHead); it.Valid(); it.Next() { - assert.NotNil(t, it.Handler()) - results[it.Method()] = append(results[it.method], it.Path()) + for method, route := range it.Prefix(seqOf(http.MethodHead), "/") { + assert.NotNil(t, route) + results[method] = append(results[method], route.Path()) } assert.Len(t, results, 1) assert.ElementsMatch(t, routesCases, results[http.MethodHead]) } -func TestIterator_SeekPrefix(t *testing.T) { +func TestIter_Prefix(t *testing.T) { tree := New().Tree() for _, rte := range routesCases { require.NoError(t, tree.Handle(http.MethodGet, rte, emptyHandler)) @@ -66,10 +154,10 @@ func TestIterator_SeekPrefix(t *testing.T) { want := []string{"/foo/bar/{baz}", "/foo/bar/{baz}/{name}"} results := make(map[string][]string) - it := NewIterator(tree) - for it.SeekPrefix("/foo"); it.Valid(); it.Next() { - assert.NotNil(t, it.Handler()) - results[it.Method()] = append(results[it.method], it.Path()) + it := tree.Iter() + for method, route := range it.Prefix(it.Methods(), "/foo") { + assert.NotNil(t, route) + results[method] = append(results[method], route.Path()) } for key := range results { @@ -77,7 +165,19 @@ func TestIterator_SeekPrefix(t *testing.T) { } } -func TestIterator_SeekMethodPrefix(t *testing.T) { +func TestIter_EdgeCase(t *testing.T) { + tree := New().Tree() + it := tree.Iter() + + assert.Empty(t, slices.Collect(left(it.Prefix(seqOf("GET"), "/")))) + assert.Empty(t, slices.Collect(left(it.Prefix(seqOf("CONNECT"), "/")))) + assert.Empty(t, slices.Collect(left(it.Reverse(seqOf("GET"), "/")))) + assert.Empty(t, slices.Collect(left(it.Reverse(seqOf("CONNECT"), "/")))) + assert.Empty(t, slices.Collect(left(it.Routes(seqOf("GET"), "/")))) + assert.Empty(t, slices.Collect(left(it.Routes(seqOf("CONNECT"), "/")))) +} + +func TestIter_PrefixWithMethod(t *testing.T) { tree := New().Tree() for _, rte := range routesCases { require.NoError(t, tree.Handle(http.MethodGet, rte, emptyHandler)) @@ -88,36 +188,104 @@ func TestIterator_SeekMethodPrefix(t *testing.T) { want := []string{"/foo/bar/{baz}", "/foo/bar/{baz}/{name}"} results := make(map[string][]string) - it := NewIterator(tree) - for it.SeekMethodPrefix(http.MethodHead, "/foo"); it.Valid(); it.Next() { - results[it.Method()] = append(results[it.method], it.Path()) + it := tree.Iter() + for method, route := range it.Prefix(seqOf(http.MethodHead), "/foo") { + assert.NotNil(t, route) + results[method] = append(results[method], route.Path()) } assert.Len(t, results, 1) assert.ElementsMatch(t, want, results[http.MethodHead]) } -func ExampleNewIterator() { - r := New() - it := NewIterator(r.Tree()) +func BenchmarkIter_Methods(b *testing.B) { + f := New() + for _, route := range staticRoutes { + require.NoError(b, f.Tree().Handle(route.method, route.path, emptyHandler)) + } + it := f.Iter() + + b.ResetTimer() + b.ReportAllocs() + + for range b.N { + for _ = range it.Methods() { + + } + } +} + +func BenchmarkIter_Reverse(b *testing.B) { + f := New() + for _, route := range githubAPI { + require.NoError(b, f.Tree().Handle(route.method, route.path, emptyHandler)) + } + it := f.Iter() + + b.ResetTimer() + b.ReportAllocs() + + for range b.N { + for _, _ = range it.Reverse(it.Methods(), "/user/subscriptions/fox/fox") { + + } + } +} + +func BenchmarkIter_Route(b *testing.B) { + f := New() + for _, route := range githubAPI { + require.NoError(b, f.Tree().Handle(route.method, route.path, emptyHandler)) + } + it := f.Iter() + + b.ResetTimer() + b.ReportAllocs() - // Iterate over all routes - for it.Rewind(); it.Valid(); it.Next() { - fmt.Println(it.Method(), it.Path()) + for range b.N { + for _, _ = range it.Reverse(it.Methods(), "/user/subscriptions/{owner}/{repo}") { + + } } +} - // Iterate over all routes for the GET method - for it.SeekMethod(http.MethodGet); it.Valid(); it.Next() { - fmt.Println(it.Method(), it.Path()) +func BenchmarkIter_Prefix(b *testing.B) { + f := New() + for _, route := range githubAPI { + require.NoError(b, f.Tree().Handle(route.method, route.path, emptyHandler)) } + it := f.Iter() - // Iterate over all routes starting with /users - for it.SeekPrefix("/users"); it.Valid(); it.Next() { - fmt.Println(it.Method(), it.Path()) + b.ResetTimer() + b.ReportAllocs() + + for range b.N { + for _, _ = range it.Prefix(it.Methods(), "/") { + + } + } +} + +func ExampleIter_All() { + f := New() + it := f.Iter() + for method, route := range it.All() { + fmt.Println(method, route.Path()) } +} + +func ExampleIter_Methods() { + f := New() + it := f.Iter() + for method := range it.Methods() { + fmt.Println(method) + } +} - // Iterate over all route starting with /users for the GET method - for it.SeekMethodPrefix(http.MethodGet, "/user"); it.Valid(); it.Next() { - fmt.Println(it.Method(), it.Path()) +func ExampleIter_Prefix() { + f := New() + it := f.Iter() + for method, route := range it.Prefix(slices.Values([]string{"GET", "POST"}), "/foo") { + fmt.Println(method, route.Path()) } } diff --git a/tree.go b/tree.go index 7bd437c..74d8d3f 100644 --- a/tree.go +++ b/tree.go @@ -8,7 +8,6 @@ import ( "fmt" "net/http" "slices" - "sort" "strings" "sync" "sync/atomic" @@ -130,12 +129,12 @@ func (t *Tree) Route(method, path string) *Route { return nil } -// Match perform a reverse lookup on the tree for the given method and path and return the matching registered route +// Reverse perform a reverse lookup on the tree for the given method and path and return the matching registered route // (if any) along with a boolean indicating if the route was matched by adding or removing a trailing slash -// (trailing slash action is recommended). This function is safe for concurrent use by multiple goroutine and while +// (trailing slash action recommended). This function is safe for concurrent use by multiple goroutine and while // mutation on Tree are ongoing. See also Tree.Lookup as an alternative. // This API is EXPERIMENTAL and is likely to change in future release. -func (t *Tree) Match(method, path string) (route *Route, tsr bool) { +func (t *Tree) Reverse(method, path string) (route *Route, tsr bool) { nds := *t.nodes.Load() index := findRootNode(method, nds) if index < 0 { @@ -152,47 +151,9 @@ func (t *Tree) Match(method, path string) (route *Route, tsr bool) { return nil, false } -// Methods returns a sorted list of HTTP methods associated with a given path in the routing tree. If the path is "*", -// it returns all HTTP methods that have at least one route registered in the tree. For a specific path, it returns the -// methods that can route requests to that path. When WithIgnoreTrailingSlash or WithRedirectTrailingSlash are enabled, -// Methods will match a registered route regardless of an extra or missing trailing slash. This function is safe for -// concurrent use by multiple goroutine and while mutation on Tree are ongoing. -// This API is EXPERIMENTAL and is likely to change in future release. -func (t *Tree) Methods(path string) []string { - var methods []string - nds := *t.nodes.Load() - - if path == "*" { - for i := range nds { - if len(nds[i].children) > 0 { - if methods == nil { - methods = make([]string, 0) - } - methods = append(methods, nds[i].key) - } - } - } else { - c := t.ctx.Get().(*cTx) - c.resetNil() - for i := range nds { - n, tsr := t.lookup(nds[i], path, c, true) - if n != nil && (!tsr || n.route.redirectTrailingSlash || n.route.ignoreTrailingSlash) { - if methods == nil { - methods = make([]string, 0) - } - methods = append(methods, nds[i].key) - } - } - c.Close() - } - - sort.Strings(methods) - return methods -} - // Lookup performs a manual route lookup for a given http.Request, returning the matched Route along with a // ContextCloser, and a boolean indicating if the route was matched by adding or removing a trailing slash -// (trailing slash action is recommended). The ContextCloser should always be closed if non-nil. This method is primarily +// (trailing slash action recommended). The ContextCloser should always be closed if non-nil. This method is primarily // intended for integrating the fox router into custom routing solutions or middleware. This function is safe for concurrent // use by multiple goroutine and while mutation on Tree are ongoing. If there is a direct match or a tsr is possible, // Lookup always return a Route and a ContextCloser. @@ -224,6 +185,13 @@ func (t *Tree) Lookup(w ResponseWriter, r *http.Request) (route *Route, cc Conte return nil, nil, tsr } +// Iter returns an iterator that provides access to a collection of iterators for traversing the routing tree. +// This function is safe for concurrent use by multiple goroutines and can operate while the Tree is being modified. +// This API is EXPERIMENTAL and may change in future releases. +func (t *Tree) Iter() Iter { + return Iter{t: t} +} + // 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 {