Skip to content

Commit

Permalink
[RFC] Rework on iterator using go 1.23 range over function (#38)
Browse files Browse the repository at this point in the history
* feat: wip per route options

* feat: wip per route options

* feat: wip per route options

* Revert "feat: wip per route options"

This reverts commit fb7f2e1.

* 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
  • Loading branch information
tigerwill90 authored Oct 9, 2024
1 parent 07edc7e commit 7f4be76
Show file tree
Hide file tree
Showing 7 changed files with 430 additions and 313 deletions.
38 changes: 7 additions & 31 deletions fox.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
package fox

import (
"errors"
"fmt"
"net"
"net/http"
Expand Down Expand Up @@ -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
Expand Down
48 changes: 20 additions & 28 deletions fox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"net/http/httptest"
"reflect"
"regexp"
"slices"
"strings"
"sync"
"sync/atomic"
Expand Down Expand Up @@ -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()))
Expand All @@ -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)
}

Expand All @@ -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)
}

Expand Down Expand Up @@ -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)
}))
}
Expand Down Expand Up @@ -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)
}))
}
Expand Down Expand Up @@ -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)
}))
}
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}

Expand Down Expand Up @@ -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}
}

Expand Down
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -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 (
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
Loading

0 comments on commit 7f4be76

Please sign in to comment.