From 0c0e6fd5634d803b87fec36c6d73a74764c8e7b6 Mon Sep 17 00:00:00 2001 From: Sylvain Muller Date: Mon, 14 Oct 2024 19:50:18 +0200 Subject: [PATCH] Polishing API (#42) * feat: returns registered route on handler & update * docs: update README.md --- README.md | 99 ++++++++++++------------------ context.go | 48 +++++++-------- context_test.go | 14 +++-- fox.go | 96 ++++++++++++++++------------- fox_test.go | 154 +++++++++++++++++++++++++---------------------- iter_test.go | 56 ++++++++--------- logger_test.go | 8 +-- recovery_test.go | 8 +-- route_test.go | 2 +- tree.go | 98 ++++++++++++++++++------------ 10 files changed, 304 insertions(+), 279 deletions(-) diff --git a/README.md b/README.md index 6e3fa38..9e531a3 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ routing structure based on user input, configuration changes, or other runtime e The current api is not yet stabilize. Breaking changes may occur before `v1.0.0` and will be noted on the release note. ## Features -**Runtime updates:** Register, update and remove route handler safely at any time without impact on performance. Fox never block while serving +**Runtime updates:** Register, update and delete route handler safely at any time without impact on performance. Fox never block while serving request! **Wildcard pattern:** Route can be registered using wildcard parameters. The matched path segment can then be easily retrieved by @@ -53,33 +53,22 @@ go get -u github.com/tigerwill90/fox package main import ( + "errors" "github.com/tigerwill90/fox" "log" "net/http" ) -type Greeting struct { - Say string -} - -func (h *Greeting) Greet(c fox.Context) { - _ = c.String(http.StatusOK, "%s %s\n", h.Say, c.Param("name")) -} - func main() { f := fox.New(fox.DefaultOptions()) - err := f.Handle(http.MethodGet, "/", func(c fox.Context) { - _ = c.String(http.StatusOK, "Welcome\n") + f.MustHandle(http.MethodGet, "/hello/{name}", func(c fox.Context) { + _ = c.String(http.StatusOK, "hello %s\n", c.Param("name")) }) - if err != nil { - panic(err) - } - h := Greeting{Say: "Hello"} - f.MustHandle(http.MethodGet, "/hello/{name}", h.Greet) - - log.Fatalln(http.ListenAndServe(":8080", f)) + if err := http.ListenAndServe(":8080", f); err != nil && !errors.Is(err, http.ErrServerClosed) { + log.Fatalln(err) + } } ```` #### Error handling @@ -109,10 +98,10 @@ 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 match +/avengers/thor match +/avengers/hulk/angry no match +/avengers/ no match Pattern /users/uuid:{id} @@ -206,7 +195,7 @@ As such threads that route requests should never encounter latency due to ongoin ### Managing routes a runtime #### Routing mutation -In this example, the handler for `routes/{action}` allow to dynamically register, update and remove handler for the +In this example, the handler for `routes/{action}` allow to dynamically register, update and delete handler for the given route and method. Thanks to Fox's design, those actions are perfectly safe and may be executed concurrently. ````go @@ -214,6 +203,7 @@ package main import ( "encoding/json" + "errors" "fmt" "github.com/tigerwill90/fox" "log" @@ -241,15 +231,15 @@ func Action(c fox.Context) { action := c.Param("action") switch action { case "add": - err = c.Fox().Handle(method, path, func(c fox.Context) { + _, err = c.Fox().Handle(method, path, func(c fox.Context) { _ = c.String(http.StatusOK, text) }) case "update": - err = c.Fox().Update(method, path, func(c fox.Context) { + _, err = c.Fox().Update(method, path, func(c fox.Context) { _ = c.String(http.StatusOK, text) }) case "delete": - err = c.Fox().Remove(method, path) + err = c.Fox().Delete(method, path) default: http.Error(c.Writer(), fmt.Sprintf("action %q is not allowed", action), http.StatusBadRequest) return @@ -263,9 +253,12 @@ func Action(c fox.Context) { } func main() { - f := fox.New() + f := fox.New(fox.DefaultOptions()) f.MustHandle(http.MethodPost, "/routes/{action}", Action) - log.Fatalln(http.ListenAndServe(":8080", f)) + + if err := http.ListenAndServe(":8080", f); err != nil && !errors.Is(err, http.ErrServerClosed) { + log.Fatalln(err) + } } ```` @@ -277,48 +270,36 @@ Note that router's options apply automatically on the new tree. package main import ( - "fox-by-example/db" + "errors" "github.com/tigerwill90/fox" - "html/template" "log" "net/http" - "strings" "time" ) -type HtmlRenderer struct { - Template template.HTML -} - -func (h *HtmlRenderer) Render(c fox.Context) { - log.Printf("matched handler path: %s", c.Path()) - _ = c.Stream( - http.StatusOK, - fox.MIMETextHTMLCharsetUTF8, - strings.NewReader(string(h.Template)), - ) -} - -func main() { - f := fox.New() - go Reload(f) - log.Fatalln(http.ListenAndServe(":8080", f)) -} - func Reload(r *fox.Router) { for ; true; <-time.Tick(10 * time.Second) { - routes := db.GetRoutes() + routes := GetRoutes() tree := r.NewTree() for _, rte := range routes { - h := HtmlRenderer{Template: rte.Template} - if err := tree.Handle(rte.Method, rte.Path, h.Render); err != nil { - log.Printf("error reloading route: %s\n", err) + if _, err := tree.Handle(rte.Method, rte.Path, rte.Handler); err != nil { + log.Printf("failed to register route: %s\n", err) continue } } // Swap the currently in-use routing tree with the new provided. r.Swap(tree) - log.Println("route reloaded") + log.Println("route reloaded!") + } +} + +func main() { + f := fox.New(fox.DefaultOptions()) + + go Reload(f) + + if err := http.ListenAndServe(":8080", f); err != nil && !errors.Is(err, http.ErrServerClosed) { + log.Fatalln(err) } } ```` @@ -333,7 +314,7 @@ is already registered for the provided method and path. By locking the `Tree`, t atomicity, as it prevents other threads from modifying the tree between the lookup and the write operation. Note that all read operation on the tree remain lock-free. ````go -func Upsert(t *fox.Tree, method, path string, handler fox.HandlerFunc) error { +func Upsert(t *fox.Tree, method, path string, handler fox.HandlerFunc) (*fox.Route, error) { t.Lock() defer t.Unlock() if t.Has(method, path) { @@ -446,9 +427,9 @@ only for 404 or 405 handlers. Possible scopes include `fox.RouteHandlers` (regul ````go f := fox.New( - fox.WithNoMethod(true), - fox.WithMiddlewareFor(fox.RouteHandlers, fox.Recovery(), Logger), - fox.WithMiddlewareFor(fox.NoRouteHandler|fox.NoMethodHandler, SpecialLogger), + fox.WithNoMethod(true), + fox.WithMiddlewareFor(fox.RouteHandler, fox.Recovery(), Logger), + fox.WithMiddlewareFor(fox.NoRouteHandler|fox.NoMethodHandler, SpecialLogger), ) ```` diff --git a/context.go b/context.go index 0e587b6..a475c37 100644 --- a/context.go +++ b/context.go @@ -55,7 +55,7 @@ type Context interface { ClientIP() (*net.IPAddr, error) // Path returns the registered path or an empty string if the handler is called in a scope other than [RouteHandler]. Path() string - // Route returns the registered route or nil if the handler is called in a scope other than [RouteHandler]. + // Route returns the registered [Route] or nil if the handler is called in a scope other than [RouteHandler]. Route() *Route // Params returns a range iterator over the matched wildcard parameters for the current route. Params() iter.Seq[Param] @@ -101,7 +101,7 @@ type Context interface { Rehydrate(route *Route) bool } -// cTx holds request-related information and allows interaction with the ResponseWriter. +// cTx holds request-related information and allows interaction with the [ResponseWriter]. type cTx struct { w ResponseWriter req *http.Request @@ -120,7 +120,7 @@ type cTx struct { tsr bool } -// Reset resets the Context to its initial state, attaching the provided ResponseWriter and http.Request. +// Reset resets the [Context] to its initial state, attaching the provided [ResponseWriter] and [http.Request]. func (c *cTx) Reset(w ResponseWriter, r *http.Request) { c.req = r c.w = w @@ -131,8 +131,8 @@ func (c *cTx) Reset(w ResponseWriter, r *http.Request) { *c.params = (*c.params)[:0] } -// Rehydrate updates the current Context to serve the provided Route, bypassing the need for a full tree lookup. -// It succeeds only if the Request's URL path strictly matches the given Route. If successful, the internal state +// Rehydrate updates the current [Context] to serve the provided [Route], bypassing the need for a full tree lookup. +// It succeeds only if the [http.Request]'s URL path strictly matches the given [Route]. If successful, the internal state // of the context is updated, allowing the context to serve the route directly, regardless of whether the route // still exists in the routing tree. This provides a key advantage in concurrent scenarios where routes may be // modified by other threads, as Rehydrate guarantees success if the path matches, without requiring serial execution @@ -167,9 +167,9 @@ func (c *cTx) Rehydrate(route *Route) bool { return true } -// reset resets the Context to its initial state, attaching the provided http.ResponseWriter and http.Request. -// Caution: always pass the original http.ResponseWriter to this method, not the ResponseWriter itself, to -// avoid wrapping the ResponseWriter within itself. Use wisely! +// reset resets the [Context] to its initial state, attaching the provided [http.ResponseWriter] and [http.Request]. +// Caution: always pass the original [http.ResponseWriter] to this method, not the [ResponseWriter] itself, to +// avoid wrapping the [ResponseWriter] within itself. Use wisely! func (c *cTx) reset(w http.ResponseWriter, r *http.Request) { c.rec.reset(w) c.req = r @@ -188,27 +188,27 @@ func (c *cTx) resetNil() { *c.params = (*c.params)[:0] } -// Request returns the *http.Request. +// Request returns the [http.Request]. func (c *cTx) Request() *http.Request { return c.req } -// SetRequest sets the *http.Request. +// SetRequest sets the [http.Request]. func (c *cTx) SetRequest(r *http.Request) { c.req = r } -// Writer returns the ResponseWriter. +// Writer returns the [ResponseWriter]. func (c *cTx) Writer() ResponseWriter { return c.w } -// SetWriter sets the ResponseWriter. +// SetWriter sets the [ResponseWriter]. func (c *cTx) SetWriter(w ResponseWriter) { c.w = w } -// RemoteIP parses the IP from Request.RemoteAddr, normalizes it, and returns a *net.IPAddr. +// RemoteIP parses the IP from [http.Request.RemoteAddr], normalizes it, and returns a [net.IPAddr]. // It never returns nil, even if parsing the IP fails. func (c *cTx) RemoteIP() *net.IPAddr { ipStr, _, _ := net.SplitHostPort(c.req.RemoteAddr) @@ -226,9 +226,9 @@ func (c *cTx) RemoteIP() *net.IPAddr { return ipAddr } -// ClientIP returns the "real" client IP address based on the configured ClientIPStrategy. -// The strategy is set using the WithClientIPStrategy option. If no strategy is configured, -// the method returns error ErrNoClientIPStrategy. +// ClientIP returns the "real" client IP address based on the configured [ClientIPStrategy]. +// The strategy is set using the [WithClientIPStrategy] option. If no strategy is configured, +// the method returns error [ErrNoClientIPStrategy]. // // The strategy used must be chosen and tuned for your network configuration. This should result // in the strategy never returning an error -- i.e., never failing to find a candidate for the "real" IP. @@ -264,7 +264,6 @@ func (c *cTx) Params() iter.Seq[Param] { } // Param retrieve a matching wildcard segment by name. -// It's a helper for c.Params.Get(name). func (c *cTx) Param(name string) string { for p := range c.Params() { if p.Key == name { @@ -274,15 +273,12 @@ func (c *cTx) Param(name string) string { return "" } -// QueryParams parses RawQuery and returns the corresponding values. -// It's a helper for c.Request.URL.Query(). Note that the parsed -// result is cached. +// QueryParams parses the [http.Request] raw query and returns the corresponding values. func (c *cTx) QueryParams() url.Values { return c.getQueries() } // QueryParam returns the first value associated with the given key. -// It's a helper for c.QueryParams().Get(name). func (c *cTx) QueryParam(name string) string { return c.getQueries().Get(name) } @@ -297,7 +293,7 @@ func (c *cTx) Header(key string) string { return c.req.Header.Get(key) } -// Path returns the registered path or an empty string if the handler is called in a scope other than RouteHandler. +// Path returns the registered path or an empty string if the handler is called in a scope other than [RouteHandler]. func (c *cTx) Path() string { if c.route == nil { return "" @@ -305,7 +301,7 @@ func (c *cTx) Path() string { return c.route.path } -// Route returns the registered route or nil if the handler is called in a scope other than RouteHandler. +// Route returns the registered [Route] or nil if the handler is called in a scope other than [RouteHandler]. func (c *cTx) Route() *Route { return c.route } @@ -328,7 +324,7 @@ func (c *cTx) Blob(code int, contentType string, buf []byte) (err error) { return } -// Stream sends data from an io.Reader with the specified status code and content type. +// Stream sends data from an [io.Reader] with the specified status code and content type. func (c *cTx) Stream(code int, contentType string, r io.Reader) (err error) { c.w.Header().Set(HeaderContentType, contentType) c.w.WriteHeader(code) @@ -345,12 +341,12 @@ func (c *cTx) Redirect(code int, url string) error { return nil } -// Tree is a local copy of the Tree in use to serve the request. +// Tree is a local copy of the [Tree] in use to serve the request. func (c *cTx) Tree() *Tree { return c.tree } -// Fox returns the Router instance. +// Fox returns the [Router] instance. func (c *cTx) Fox() *Router { return c.fox } diff --git a/context_test.go b/context_test.go index a0e1aaa..f3946c5 100644 --- a/context_test.go +++ b/context_test.go @@ -392,9 +392,9 @@ func TestContext_Fox(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/foo", nil) f := New() - require.NoError(t, f.Handle(http.MethodGet, "/foo", func(c Context) { + require.NoError(t, onlyError(f.Handle(http.MethodGet, "/foo", func(c Context) { assert.NotNil(t, c.Fox()) - })) + }))) f.ServeHTTP(w, req) } @@ -405,9 +405,9 @@ func TestContext_Tree(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/foo", nil) f := New() - require.NoError(t, f.Handle(http.MethodGet, "/foo", func(c Context) { + require.NoError(t, onlyError(f.Handle(http.MethodGet, "/foo", func(c Context) { assert.NotNil(t, c.Tree()) - })) + }))) f.ServeHTTP(w, req) } @@ -433,9 +433,11 @@ func TestContext_Scope(t *testing.T) { assert.Equal(t, OptionsHandler, c.Scope()) }), ) - require.NoError(t, f.Handle(http.MethodGet, "/foo", func(c Context) { + + _, err := f.Handle(http.MethodGet, "/foo", func(c Context) { assert.Equal(t, RouteHandler, c.Scope()) - })) + }) + require.NoError(t, err) cases := []struct { name string diff --git a/fox.go b/fox.go index f9613da..d9339da 100644 --- a/fox.go +++ b/fox.go @@ -27,20 +27,20 @@ var ( ) // HandlerFunc is a function type that responds to an HTTP request. -// It enforces the same contract as http.Handler but provides additional feature -// like matched wildcard route segments via the Context type. The Context is freed once +// It enforces the same contract as [http.Handler] but provides additional feature +// like matched wildcard route segments via the [Context] type. The [Context] is freed once // the HandlerFunc returns and may be reused later to save resources. If you need -// to hold the context longer, you have to copy it (see Clone method). +// to hold the context longer, you have to copy it (see [Context.Clone] method). // -// Similar to http.Handler, to abort a HandlerFunc so the client sees an interrupted -// response, panic with the value http.ErrAbortHandler. +// Similar to [http.Handler], to abort a HandlerFunc so the client sees an interrupted +// response, panic with the value [http.ErrAbortHandler]. // // HandlerFunc functions should be thread-safe, as they will be called concurrently. type HandlerFunc func(c Context) -// MiddlewareFunc is a function type for implementing HandlerFunc middleware. -// The returned HandlerFunc usually wraps the input HandlerFunc, allowing you to perform operations -// before and/or after the wrapped HandlerFunc is executed. MiddlewareFunc functions should +// MiddlewareFunc is a function type for implementing [HandlerFunc] middleware. +// The returned [HandlerFunc] usually wraps the input [HandlerFunc], allowing you to perform operations +// before and/or after the wrapped [HandlerFunc] is executed. MiddlewareFunc functions should // be thread-safe, as they will be called concurrently. type MiddlewareFunc func(next HandlerFunc) HandlerFunc @@ -57,8 +57,8 @@ type ClientIPStrategy interface { ClientIP(c Context) (*net.IPAddr, error) } -// The ClientIPStrategyFunc type is an adapter to allow the use of ordinary functions as ClientIPStrategy. If f is a function -// with the appropriate signature, ClientIPStrategyFunc(f) is a ClientIPStrategyFunc that calls f. +// The ClientIPStrategyFunc type is an adapter to allow the use of ordinary functions as [ClientIPStrategy]. If f is a +// function with the appropriate signature, ClientIPStrategyFunc(f) is a ClientIPStrategyFunc that calls f. type ClientIPStrategyFunc func(c Context) (*net.IPAddr, error) // ClientIP calls f(c). @@ -166,9 +166,9 @@ func (fox *Router) ClientIPStrategyEnabled() bool { return !ok } -// NewTree returns a fresh routing Tree that inherits all registered router options. It's safe to create multiple Tree +// NewTree returns a fresh routing [Tree] that inherits all registered router options. It's safe to create multiple [Tree] // concurrently. However, a Tree itself is not thread-safe and all its APIs that perform write operations should be run -// serially. Note that a Tree give direct access to the underlying sync.Mutex. +// serially. Note that a [Tree] give direct access to the underlying [sync.Mutex]. // This API is EXPERIMENTAL and is likely to change in future release. func (fox *Router) NewTree() *Tree { tree := new(Tree) @@ -210,53 +210,65 @@ func (fox *Router) Swap(new *Tree) (old *Tree) { return fox.tree.Swap(new) } -// Handle registers a new handler for the given method and path. This function return an error if the route -// is already registered or conflict with another. It's perfectly safe to add a new handler while the tree is in use -// for serving requests. This function is safe for concurrent use by multiple goroutine. -// To override an existing route, use Update. -func (fox *Router) Handle(method, path string, handler HandlerFunc, opts ...PathOption) error { +// Handle registers a new handler for the given method and path. On success, it returns the newly registered [Route]. +// If an error occurs, it returns one of the following: +// - [ErrRouteExist]: If the route is already registered. +// - [ErrRouteConflict]: If the route conflicts with another. +// - [ErrInvalidRoute]: If the provided method or path is invalid. +// +// It's safe to add a new handler while the tree is in use for serving requests. This function is safe for concurrent +// use by multiple goroutine. To override an existing route, use [Router.Update]. +func (fox *Router) Handle(method, path string, handler HandlerFunc, opts ...PathOption) (*Route, error) { t := fox.Tree() t.Lock() defer t.Unlock() return t.Handle(method, path, handler, opts...) } -// MustHandle registers a new handler for the given method and path. This function is a convenience -// wrapper for the Handle function. It will panic if the route is already registered or conflicts -// with another route. It's perfectly safe to add a new handler while the tree is in use for serving -// requests. This function is safe for concurrent use by multiple goroutines. -// To override an existing route, use Update. -func (fox *Router) MustHandle(method, path string, handler HandlerFunc, opts ...PathOption) { - if err := fox.Handle(method, path, handler, opts...); err != nil { +// MustHandle registers a new handler for the given method and path. On success, it returns the newly registered [Route] +// This function is a convenience wrapper for the [Router.Handle] function and panics on error. It's perfectly safe to +// add a new handler while the tree is in use for serving requests. This function is safe for concurrent use by multiple +// goroutines. To override an existing route, use [Router.Update]. +func (fox *Router) MustHandle(method, path string, handler HandlerFunc, opts ...PathOption) *Route { + rte, err := fox.Handle(method, path, handler, opts...) + if err != nil { panic(err) } + return rte } -// Update override an existing handler for the given method and path. If the route does not exist, -// the function return an ErrRouteNotFound. It's perfectly safe to update a handler while the tree is in use for -// serving requests. This function is safe for concurrent use by multiple goroutine. -// To add new handler, use Handle method. -func (fox *Router) Update(method, path string, handler HandlerFunc, opts ...PathOption) error { +// Update override an existing handler for the given method and path. On success, it returns the newly registered [Route]. +// If an error occurs, it returns one of the following: +// - [ErrRouteNotFound]: if the route does not exist. +// - [ErrInvalidRoute]: If the provided method or path is invalid. +// +// It's safe to update a handler while the tree is in use for serving requests. This function is safe for concurrent +// use by multiple goroutine. To add new handler, use [Router.Handle] method. +func (fox *Router) Update(method, path string, handler HandlerFunc, opts ...PathOption) (*Route, error) { t := fox.Tree() t.Lock() defer t.Unlock() return t.Update(method, path, handler, opts...) } -// Remove delete an existing handler for the given method and path. If the route does not exist, the function -// return an ErrRouteNotFound. It's perfectly safe to remove a handler while the tree is in use for serving requests. -// This function is safe for concurrent use by multiple goroutine. -func (fox *Router) Remove(method, path string) error { +// Delete deletes an existing handler for the given method and path. If an error occurs, it returns one of the following: +// - [ErrRouteNotFound]: if the route does not exist. +// - [ErrInvalidRoute]: If the provided method or path is invalid. +// +// It's safe to delete a handler while the tree is in use for serving requests. This function is safe for concurrent +// use by multiple goroutine. +func (fox *Router) Delete(method, path string) error { t := fox.Tree() t.Lock() defer t.Unlock() - return t.Remove(method, path) + return t.Delete(method, path) } -// Lookup is a helper that calls Tree.Lookup. For more details, refer to Tree.Lookup. -// It performs a manual route lookup for a given http.Request, returning the matched Route along with a ContextCloser, -// and a boolean indicating if a trailing slash action (e.g. redirect) is recommended (tsr). The ContextCloser should always -// be closed if non-nil. +// 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 recommended). If there is a direct match or a tsr is possible, Lookup always return a +// [Route] and a [ContextCloser]. The [ContextCloser] should always be closed if non-nil. This function is safe for +// concurrent use by multiple goroutine and while mutation on [Tree] are ongoing. See also [Tree.Reverse] as an alternative. // 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() @@ -264,26 +276,26 @@ func (fox *Router) Lookup(w ResponseWriter, r *http.Request) (route *Route, cc C } // 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 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 +// DefaultNotFoundHandler is a simple [HandlerFunc] that replies to each request // with a “404 page not found” reply. func DefaultNotFoundHandler(c Context) { http.Error(c.Writer(), "404 page not found", http.StatusNotFound) } -// DefaultMethodNotAllowedHandler is a simple HandlerFunc that replies to each request +// DefaultMethodNotAllowedHandler is a simple [HandlerFunc] that replies to each request // with a “405 Method Not Allowed” reply. func DefaultMethodNotAllowedHandler(c Context) { http.Error(c.Writer(), http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed) } -// DefaultOptionsHandler is a simple HandlerFunc that replies to each request with a "200 OK" reply. +// DefaultOptionsHandler is a simple [HandlerFunc] that replies to each request with a "200 OK" reply. func DefaultOptionsHandler(c Context) { c.Writer().WriteHeader(http.StatusOK) } diff --git a/fox_test.go b/fox_test.go index 351dea0..881f802 100644 --- a/fox_test.go +++ b/fox_test.go @@ -486,7 +486,7 @@ func benchRouteParallel(b *testing.B, router http.Handler, rte route) { func BenchmarkStaticAll(b *testing.B) { r := New() for _, route := range staticRoutes { - require.NoError(b, r.Tree().Handle(route.method, route.path, emptyHandler)) + require.NoError(b, onlyError(r.Tree().Handle(route.method, route.path, emptyHandler))) } benchRoutes(b, r, staticRoutes) @@ -495,7 +495,7 @@ func BenchmarkStaticAll(b *testing.B) { func BenchmarkGithubParamsAll(b *testing.B) { r := New() for _, route := range githubAPI { - require.NoError(b, r.Tree().Handle(route.method, route.path, emptyHandler)) + require.NoError(b, onlyError(r.Tree().Handle(route.method, route.path, emptyHandler))) } req := httptest.NewRequest(http.MethodGet, "/repos/sylvain/fox/hooks/1500", nil) @@ -526,7 +526,7 @@ func BenchmarkLongParam(b *testing.B) { func BenchmarkOverlappingRoute(b *testing.B) { r := New() for _, route := range overlappingRoutes { - require.NoError(b, r.Tree().Handle(route.method, route.path, emptyHandler)) + require.NoError(b, onlyError(r.Tree().Handle(route.method, route.path, emptyHandler))) } req := httptest.NewRequest(http.MethodGet, "/foo/abc/id:123/xy", nil) @@ -562,14 +562,14 @@ func BenchmarkWithIgnoreTrailingSlash(b *testing.B) { func BenchmarkStaticParallel(b *testing.B) { r := New() for _, route := range staticRoutes { - require.NoError(b, r.Tree().Handle(route.method, route.path, emptyHandler)) + require.NoError(b, onlyError(r.Tree().Handle(route.method, route.path, emptyHandler))) } benchRouteParallel(b, r, route{http.MethodGet, "/progs/image_package4.out"}) } func BenchmarkCatchAll(b *testing.B) { r := New() - require.NoError(b, r.Tree().Handle(http.MethodGet, "/something/*{args}", emptyHandler)) + require.NoError(b, onlyError(r.Tree().Handle(http.MethodGet, "/something/*{args}", emptyHandler))) w := new(mockResponseWriter) req := httptest.NewRequest(http.MethodGet, "/something/awesome", nil) @@ -583,7 +583,7 @@ func BenchmarkCatchAll(b *testing.B) { func BenchmarkCatchAllParallel(b *testing.B) { r := New() - require.NoError(b, r.Tree().Handle(http.MethodGet, "/something/*{args}", emptyHandler)) + require.NoError(b, onlyError(r.Tree().Handle(http.MethodGet, "/something/*{args}", emptyHandler))) w := new(mockResponseWriter) req := httptest.NewRequest("GET", "/something/awesome", nil) @@ -616,7 +616,7 @@ func TestStaticRoute(t *testing.T) { r := New() for _, route := range staticRoutes { - require.NoError(t, r.Tree().Handle(route.method, route.path, pathHandler)) + require.NoError(t, onlyError(r.Tree().Handle(route.method, route.path, pathHandler))) } for _, route := range staticRoutes { @@ -632,7 +632,7 @@ func TestStaticRouteMalloc(t *testing.T) { r := New() for _, route := range staticRoutes { - require.NoError(t, r.Tree().Handle(route.method, route.path, emptyHandler)) + require.NoError(t, onlyError(r.Tree().Handle(route.method, route.path, emptyHandler))) } for _, route := range staticRoutes { @@ -662,7 +662,7 @@ func TestParamsRoute(t *testing.T) { _ = c.String(200, c.Request().URL.Path) } for _, route := range githubAPI { - require.NoError(t, r.Tree().Handle(route.method, route.path, h)) + require.NoError(t, onlyError(r.Tree().Handle(route.method, route.path, h))) } for _, route := range githubAPI { req := httptest.NewRequest(route.method, route.path, nil) @@ -676,7 +676,7 @@ func TestParamsRoute(t *testing.T) { func TestParamsRouteMalloc(t *testing.T) { r := New() for _, route := range githubAPI { - require.NoError(t, r.Tree().Handle(route.method, route.path, emptyHandler)) + require.NoError(t, onlyError(r.Tree().Handle(route.method, route.path, emptyHandler))) } for _, route := range githubAPI { req := httptest.NewRequest(route.method, route.path, nil) @@ -689,7 +689,7 @@ func TestParamsRouteMalloc(t *testing.T) { func TestOverlappingRouteMalloc(t *testing.T) { r := New() for _, route := range overlappingRoutes { - require.NoError(t, r.Tree().Handle(route.method, route.path, emptyHandler)) + require.NoError(t, onlyError(r.Tree().Handle(route.method, route.path, emptyHandler))) } for _, route := range overlappingRoutes { req := httptest.NewRequest(route.method, route.path, nil) @@ -713,7 +713,7 @@ func TestRouterWildcard(t *testing.T) { } for _, route := range routes { - require.NoError(t, r.Tree().Handle(http.MethodGet, route.path, pathHandler)) + require.NoError(t, onlyError(r.Tree().Handle(http.MethodGet, route.path, pathHandler))) } for _, route := range routes { @@ -744,7 +744,7 @@ func TestRouteWithParams(t *testing.T) { "/info/{user}/project/{project}", } for _, rte := range routes { - require.NoError(t, tree.Handle(http.MethodGet, rte, emptyHandler)) + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) } nds := *tree.nodes.Load() @@ -783,7 +783,7 @@ func TestRouteParamEmptySegment(t *testing.T) { } for _, tc := range cases { - require.NoError(t, tree.Handle(http.MethodGet, tc.route, emptyHandler)) + require.NoError(t, onlyError(tree.Handle(http.MethodGet, tc.route, emptyHandler))) } for _, tc := range cases { @@ -1239,7 +1239,7 @@ func TestOverlappingRoute(t *testing.T) { t.Run(tc.name, func(t *testing.T) { tree := r.NewTree() for _, rte := range tc.routes { - require.NoError(t, tree.Handle(http.MethodGet, rte, emptyHandler)) + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) } nds := *tree.nodes.Load() @@ -1355,7 +1355,10 @@ func TestInsertConflict(t *testing.T) { t.Run(tc.name, func(t *testing.T) { tree := New().Tree() for _, rte := range tc.routes { - err := tree.Handle(http.MethodGet, rte.path, emptyHandler) + r, err := tree.Handle(http.MethodGet, rte.path, emptyHandler) + if err != nil { + assert.Nil(t, r) + } assert.ErrorIs(t, err, rte.wantErr) if cErr, ok := err.(*RouteConflictError); ok { assert.Equal(t, rte.wantMatch, cErr.Matched) @@ -1418,9 +1421,12 @@ func TestUpdateConflict(t *testing.T) { t.Run(tc.name, func(t *testing.T) { tree := New().Tree() for _, rte := range tc.routes { - require.NoError(t, tree.Handle(http.MethodGet, rte, emptyHandler)) + 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) } - err := tree.Update(http.MethodGet, tc.update, emptyHandler) assert.ErrorIs(t, err, tc.wantErr) if cErr, ok := err.(*RouteConflictError); ok { assert.Equal(t, tc.wantMatch, cErr.Matched) @@ -1432,13 +1438,13 @@ func TestUpdateConflict(t *testing.T) { func TestInvalidRoute(t *testing.T) { f := New() // Invalid route on insert - assert.ErrorIs(t, f.Handle("get", "/foo", emptyHandler), ErrInvalidRoute) - assert.ErrorIs(t, f.Handle("", "/foo", emptyHandler), ErrInvalidRoute) - assert.ErrorIs(t, f.Handle(http.MethodGet, "/foo", nil), ErrInvalidRoute) + 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, f.Update("", "/foo", emptyHandler), ErrInvalidRoute) - assert.ErrorIs(t, f.Update(http.MethodGet, "/foo", nil), ErrInvalidRoute) + 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) { @@ -1488,9 +1494,9 @@ func TestUpdateRoute(t *testing.T) { t.Run(tc.name, func(t *testing.T) { tree := New().Tree() for _, rte := range tc.routes { - require.NoError(t, tree.Handle(http.MethodGet, rte, emptyHandler)) + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) } - assert.NoError(t, tree.Update(http.MethodGet, tc.update, emptyHandler)) + assert.NoError(t, onlyError(tree.Update(http.MethodGet, tc.update, emptyHandler))) }) } } @@ -1832,9 +1838,9 @@ func TestRouterWithIgnoreTrailingSlash(t *testing.T) { r := New(WithIgnoreTrailingSlash(true)) require.True(t, r.IgnoreTrailingSlashEnabled()) for _, path := range tc.paths { - require.NoError(t, r.Tree().Handle(tc.method, path, func(c Context) { + 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()) @@ -1869,7 +1875,7 @@ func TestRouterWithClientIPStrategy(t *testing.T) { require.NotNil(t, rte) assert.True(t, rte.ClientIPStrategyEnabled()) - require.NoError(t, f.Update(http.MethodGet, "/foo", emptyHandler, WithClientIPStrategy(nil))) + 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()) @@ -2011,7 +2017,7 @@ func TestRedirectTrailingSlash(t *testing.T) { r := New(WithRedirectTrailingSlash(true)) require.True(t, r.RedirectTrailingSlashEnabled()) for _, path := range tc.paths { - require.NoError(t, r.Tree().Handle(tc.method, path, emptyHandler)) + 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()) @@ -2034,7 +2040,7 @@ func TestRedirectTrailingSlash(t *testing.T) { func TestEncodedRedirectTrailingSlash(t *testing.T) { r := New(WithRedirectTrailingSlash(true)) - require.NoError(t, r.Handle(http.MethodGet, "/foo/{bar}/", emptyHandler)) + require.NoError(t, onlyError(r.Handle(http.MethodGet, "/foo/{bar}/", emptyHandler))) req := httptest.NewRequest(http.MethodGet, "/foo/bar%2Fbaz", nil) w := httptest.NewRecorder() @@ -2158,12 +2164,12 @@ func TestRouterWithTsrParams(t *testing.T) { t.Run(tc.name, func(t *testing.T) { f := New(WithIgnoreTrailingSlash(true)) for _, rte := range tc.routes { - require.NoError(t, f.Handle(http.MethodGet, rte, func(c Context) { + 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() @@ -2174,19 +2180,19 @@ func TestRouterWithTsrParams(t *testing.T) { } -func TestTree_Remove(t *testing.T) { +func TestTree_Delete(t *testing.T) { tree := New().Tree() routes := make([]route, len(githubAPI)) copy(routes, githubAPI) for _, rte := range routes { - require.NoError(t, tree.Handle(rte.method, rte.path, emptyHandler)) + 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.Remove(rte.method, rte.path)) + require.NoError(t, tree.Delete(rte.method, rte.path)) } it := tree.Iter() @@ -2196,17 +2202,17 @@ func TestTree_Remove(t *testing.T) { assert.Equal(t, 4, len(*tree.nodes.Load())) } -func TestTree_RemoveRoot(t *testing.T) { +func TestTree_DeleteRoot(t *testing.T) { tree := New().Tree() - require.NoError(t, tree.Handle(http.MethodOptions, "/foo/bar", emptyHandler)) - require.NoError(t, tree.Remove(http.MethodOptions, "/foo/bar")) + 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_Methods(t *testing.T) { f := New() for _, rte := range githubAPI { - require.NoError(t, f.Handle(rte.method, rte.path, emptyHandler)) + 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"))) @@ -2223,8 +2229,8 @@ func TestTree_Methods(t *testing.T) { func TestTree_MethodsWithIgnoreTsEnable(t *testing.T) { f := New(WithIgnoreTrailingSlash(true)) for _, method := range []string{"DELETE", "GET", "PUT"} { - require.NoError(t, f.Handle(method, "/foo/bar", emptyHandler)) - require.NoError(t, f.Handle(method, "/john/doe/", emptyHandler)) + 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/"))) @@ -2274,7 +2280,7 @@ func TestRouterWithAllowedMethod(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { for _, method := range tc.methods { - require.NoError(t, r.Tree().Handle(method, tc.path, emptyHandler)) + require.NoError(t, onlyError(r.Tree().Handle(method, tc.path, emptyHandler))) } req := httptest.NewRequest(tc.target, tc.path, nil) w := httptest.NewRecorder() @@ -2318,7 +2324,7 @@ func TestRouterWithAllowedMethodAndIgnoreTsEnable(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { for _, method := range tc.methods { - require.NoError(t, r.Tree().Handle(method, tc.path, emptyHandler)) + require.NoError(t, onlyError(r.Tree().Handle(method, tc.path, emptyHandler))) } req := httptest.NewRequest(tc.target, tc.req, nil) w := httptest.NewRecorder() @@ -2360,7 +2366,7 @@ func TestRouterWithAllowedMethodAndIgnoreTsDisable(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { for _, method := range tc.methods { - require.NoError(t, r.Tree().Handle(method, tc.path, emptyHandler)) + require.NoError(t, onlyError(r.Tree().Handle(method, tc.path, emptyHandler))) } req := httptest.NewRequest(tc.target, tc.req, nil) w := httptest.NewRecorder() @@ -2376,7 +2382,7 @@ func TestRouterWithMethodNotAllowedHandler(t *testing.T) { c.Writer().WriteHeader(http.StatusMethodNotAllowed) })) - require.NoError(t, f.Handle(http.MethodPost, "/foo/bar", 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) @@ -2446,10 +2452,10 @@ func TestRouterWithAutomaticOptions(t *testing.T) { for _, tc := range cases { 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) { + 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() @@ -2522,10 +2528,10 @@ func TestRouterWithAutomaticOptionsAndIgnoreTsOptionEnable(t *testing.T) { for _, tc := range cases { 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) { + 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() @@ -2567,10 +2573,10 @@ func TestRouterWithAutomaticOptionsAndIgnoreTsOptionDisable(t *testing.T) { for _, tc := range cases { 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) { + 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() @@ -2589,8 +2595,8 @@ func TestRouterWithOptionsHandler(t *testing.T) { c.Writer().WriteHeader(http.StatusNoContent) })) - require.NoError(t, f.Handle(http.MethodGet, "/foo/{bar}", emptyHandler)) - require.NoError(t, f.Handle(http.MethodPost, "/foo/{bar}", emptyHandler)) + require.NoError(t, onlyError(f.Handle(http.MethodGet, "/foo/{bar}", emptyHandler))) + require.NoError(t, onlyError(f.Handle(http.MethodPost, "/foo/{bar}", emptyHandler))) req := httptest.NewRequest(http.MethodOptions, "/foo/bar", nil) w := httptest.NewRecorder() @@ -2620,7 +2626,7 @@ func TestWithScopedMiddleware(t *testing.T) { }) r := New(WithMiddlewareFor(NoRouteHandler, m)) - require.NoError(t, r.Handle(http.MethodGet, "/foo/bar", emptyHandler)) + require.NoError(t, onlyError(r.Handle(http.MethodGet, "/foo/bar", emptyHandler))) req := httptest.NewRequest(http.MethodGet, "/foo/bar", nil) w := httptest.NewRecorder() @@ -2646,7 +2652,7 @@ func TestUpdateWithMiddleware(t *testing.T) { w := httptest.NewRecorder() // Add middleware - require.NoError(t, f.Update(http.MethodGet, "/foo", emptyHandler, WithMiddleware(m))) + require.NoError(t, onlyError(f.Update(http.MethodGet, "/foo", emptyHandler, WithMiddleware(m)))) f.ServeHTTP(w, req) assert.True(t, called) called = false @@ -2661,7 +2667,7 @@ func TestUpdateWithMiddleware(t *testing.T) { called = false // Remove middleware - require.NoError(t, f.Update(http.MethodGet, "/foo", emptyHandler)) + require.NoError(t, onlyError(f.Update(http.MethodGet, "/foo", emptyHandler))) f.ServeHTTP(w, req) assert.False(t, called) called = false @@ -2746,7 +2752,7 @@ func TestWithNotFoundHandler(t *testing.T) { } f := New(WithNoRouteHandler(notFound)) - require.NoError(t, f.Handle(http.MethodGet, "/foo", emptyHandler)) + require.NoError(t, onlyError(f.Handle(http.MethodGet, "/foo", emptyHandler))) req := httptest.NewRequest(http.MethodGet, "/foo/bar", nil) w := httptest.NewRecorder() @@ -2760,7 +2766,7 @@ func TestRouter_Lookup(t *testing.T) { rx := regexp.MustCompile("({|\\*{)[A-z]+[}]") f := New() for _, rte := range githubAPI { - require.NoError(t, f.Handle(rte.method, rte.path, emptyHandler)) + require.NoError(t, onlyError(f.Handle(rte.method, rte.path, emptyHandler))) } for _, rte := range githubAPI { @@ -2808,7 +2814,7 @@ func TestTree_Has(t *testing.T) { r := New() for _, rte := range routes { - require.NoError(t, r.Handle(http.MethodGet, rte, emptyHandler)) + require.NoError(t, onlyError(r.Handle(http.MethodGet, rte, emptyHandler))) } cases := []struct { @@ -2871,7 +2877,7 @@ func TestTree_Route(t *testing.T) { r := New() for _, rte := range routes { - require.NoError(t, r.Handle(http.MethodGet, rte, emptyHandler)) + require.NoError(t, onlyError(r.Handle(http.MethodGet, rte, emptyHandler))) } cases := []struct { @@ -2930,7 +2936,7 @@ func TestTree_RouteWithIgnoreTrailingSlashEnable(t *testing.T) { r := New(WithIgnoreTrailingSlash(true)) for _, rte := range routes { - require.NoError(t, r.Handle(http.MethodGet, rte, emptyHandler)) + require.NoError(t, onlyError(r.Handle(http.MethodGet, rte, emptyHandler))) } cases := []struct { @@ -3148,10 +3154,10 @@ func TestDataRace(t *testing.T) { tree.Lock() defer tree.Unlock() if tree.Has(method, route) { - assert.NoError(t, tree.Update(method, route, h)) + assert.NoError(t, onlyError(tree.Update(method, route, h))) return } - assert.NoError(t, tree.Handle(method, route, h)) + assert.NoError(t, onlyError(tree.Handle(method, route, h))) // assert.NoError(t, r.Handle("PING", route, h)) }(rte.method, rte.path) @@ -3162,10 +3168,10 @@ func TestDataRace(t *testing.T) { tree.Lock() defer tree.Unlock() if tree.Has(method, route) { - assert.NoError(t, tree.Remove(method, route)) + assert.NoError(t, tree.Delete(method, route)) return } - assert.NoError(t, tree.Handle(method, route, newH)) + assert.NoError(t, onlyError(tree.Handle(method, route, newH))) }(rte.method, rte.path) go func(method, route string) { @@ -3205,9 +3211,9 @@ func TestConcurrentRequestHandling(t *testing.T) { _ = c.String(200, c.Path()) }) - require.NoError(t, r.Handle(http.MethodGet, "/repos/{owner}/{repo}/keys", h1)) - require.NoError(t, r.Handle(http.MethodGet, "/repos/{owner}/{repo}/contents/*{path}", h2)) - require.NoError(t, r.Handle(http.MethodGet, "/users/{user}/received_events/public", h3)) + 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) @@ -3264,7 +3270,7 @@ func atomicSync() (start func(), wait func()) { func TestNode_String(t *testing.T) { f := New() - require.NoError(t, f.Handle(http.MethodGet, "/foo/{bar}/*{baz}", emptyHandler)) + require.NoError(t, onlyError(f.Handle(http.MethodGet, "/foo/{bar}/*{baz}", emptyHandler))) tree := f.Tree() nds := *tree.nodes.Load() @@ -3340,7 +3346,7 @@ func ExampleRouter_Tree() { // 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) error { + upsert := func(method, path string, handler HandlerFunc) (*Route, error) { tree.Lock() defer tree.Unlock() if tree.Has(method, path) { @@ -3349,7 +3355,7 @@ func ExampleRouter_Tree() { return tree.Handle(method, path, handler) } - _ = upsert(http.MethodGet, "/foo/bar", func(c Context) { + _, _ = 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() @@ -3357,7 +3363,7 @@ func ExampleRouter_Tree() { }) // Bad, instead make a local copy of the tree! - _ = func(method, path string, handler HandlerFunc) error { + _ = func(method, path string, handler HandlerFunc) (*Route, error) { r.Tree().Lock() defer r.Tree().Unlock() if r.Tree().Has(method, path) { @@ -3447,3 +3453,7 @@ func ExampleTree_Has() { exist := tree.Has(http.MethodGet, "/hello/{name}") fmt.Println(exist) // true } + +func onlyError[T any](_ T, err error) error { + return err +} diff --git a/iter_test.go b/iter_test.go index 95e1a8a..78f0e71 100644 --- a/iter_test.go +++ b/iter_test.go @@ -19,9 +19,9 @@ var routesCases = []string{"/fox/router", "/foo/bar/{baz}", "/foo/bar/{baz}/{nam func TestIter_Routes(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)) + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + require.NoError(t, onlyError(tree.Handle(http.MethodPost, rte, emptyHandler))) + require.NoError(t, onlyError(tree.Handle(http.MethodHead, rte, emptyHandler))) } results := make(map[string][]string) @@ -40,9 +40,9 @@ func TestIter_Routes(t *testing.T) { 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)) + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + require.NoError(t, onlyError(tree.Handle(http.MethodPost, rte, emptyHandler))) + require.NoError(t, onlyError(tree.Handle(http.MethodHead, rte, emptyHandler))) } results := make(map[string][]string) @@ -61,9 +61,9 @@ func TestIter_All(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)) + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + require.NoError(t, onlyError(tree.Handle(http.MethodPost, rte, emptyHandler))) + require.NoError(t, onlyError(tree.Handle(http.MethodHead, rte, emptyHandler))) } var ( @@ -83,9 +83,9 @@ func TestIter_AllBreak(t *testing.T) { 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)) + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + require.NoError(t, onlyError(tree.Handle(http.MethodPost, rte, emptyHandler))) + require.NoError(t, onlyError(tree.Handle(http.MethodHead, rte, emptyHandler))) } var ( @@ -105,9 +105,9 @@ func TestIter_ReverseBreak(t *testing.T) { 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)) + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + require.NoError(t, onlyError(tree.Handle(http.MethodPost, rte, emptyHandler))) + require.NoError(t, onlyError(tree.Handle(http.MethodHead, rte, emptyHandler))) } var ( @@ -127,9 +127,9 @@ func TestIter_RouteBreak(t *testing.T) { func TestIter_RootPrefixOneMethod(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)) + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + require.NoError(t, onlyError(tree.Handle(http.MethodPost, rte, emptyHandler))) + require.NoError(t, onlyError(tree.Handle(http.MethodHead, rte, emptyHandler))) } results := make(map[string][]string) @@ -147,9 +147,9 @@ func TestIter_RootPrefixOneMethod(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)) - require.NoError(t, tree.Handle(http.MethodPost, rte, emptyHandler)) - require.NoError(t, tree.Handle(http.MethodHead, rte, emptyHandler)) + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + require.NoError(t, onlyError(tree.Handle(http.MethodPost, rte, emptyHandler))) + require.NoError(t, onlyError(tree.Handle(http.MethodHead, rte, emptyHandler))) } want := []string{"/foo/bar/{baz}", "/foo/bar/{baz}/{name}"} @@ -181,9 +181,9 @@ func TestIter_EdgeCase(t *testing.T) { func TestIter_PrefixWithMethod(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)) + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + require.NoError(t, onlyError(tree.Handle(http.MethodPost, rte, emptyHandler))) + require.NoError(t, onlyError(tree.Handle(http.MethodHead, rte, emptyHandler))) } want := []string{"/foo/bar/{baz}", "/foo/bar/{baz}/{name}"} @@ -202,7 +202,7 @@ func TestIter_PrefixWithMethod(t *testing.T) { func BenchmarkIter_Methods(b *testing.B) { f := New() for _, route := range staticRoutes { - require.NoError(b, f.Tree().Handle(route.method, route.path, emptyHandler)) + require.NoError(b, onlyError(f.Tree().Handle(route.method, route.path, emptyHandler))) } it := f.Iter() @@ -219,7 +219,7 @@ func BenchmarkIter_Methods(b *testing.B) { func BenchmarkIter_Reverse(b *testing.B) { f := New() for _, route := range githubAPI { - require.NoError(b, f.Tree().Handle(route.method, route.path, emptyHandler)) + require.NoError(b, onlyError(f.Tree().Handle(route.method, route.path, emptyHandler))) } it := f.Iter() @@ -236,7 +236,7 @@ func BenchmarkIter_Reverse(b *testing.B) { func BenchmarkIter_Route(b *testing.B) { f := New() for _, route := range githubAPI { - require.NoError(b, f.Tree().Handle(route.method, route.path, emptyHandler)) + require.NoError(b, onlyError(f.Tree().Handle(route.method, route.path, emptyHandler))) } it := f.Iter() @@ -253,7 +253,7 @@ func BenchmarkIter_Route(b *testing.B) { func BenchmarkIter_Prefix(b *testing.B) { f := New() for _, route := range githubAPI { - require.NoError(b, f.Tree().Handle(route.method, route.path, emptyHandler)) + require.NoError(b, onlyError(f.Tree().Handle(route.method, route.path, emptyHandler))) } it := f.Iter() diff --git a/logger_test.go b/logger_test.go index 3c26451..18c5144 100644 --- a/logger_test.go +++ b/logger_test.go @@ -27,12 +27,12 @@ func TestLoggerWithHandler(t *testing.T) { }, }))), ) - require.NoError(t, f.Handle(http.MethodGet, "/success", func(c Context) { + require.NoError(t, onlyError(f.Handle(http.MethodGet, "/success", func(c Context) { c.Writer().WriteHeader(http.StatusOK) - })) - require.NoError(t, f.Handle(http.MethodGet, "/failure", func(c Context) { + }))) + require.NoError(t, onlyError(f.Handle(http.MethodGet, "/failure", func(c Context) { c.Writer().WriteHeader(http.StatusInternalServerError) - })) + }))) cases := []struct { name string diff --git a/recovery_test.go b/recovery_test.go index 6791780..0d327b6 100644 --- a/recovery_test.go +++ b/recovery_test.go @@ -27,7 +27,7 @@ func TestAbortHandler(t *testing.T) { _ = c.String(200, "foo") } - require.NoError(t, r.Tree().Handle(http.MethodPost, "/{foo}", h)) + require.NoError(t, onlyError(r.Tree().Handle(http.MethodPost, "/{foo}", h))) req := httptest.NewRequest(http.MethodPost, "/foo", nil) req.Header.Set(HeaderAuthorization, "foobar") w := httptest.NewRecorder() @@ -63,7 +63,7 @@ func TestRecoveryMiddleware(t *testing.T) { _ = c.String(200, "foo") } - require.NoError(t, r.Tree().Handle(http.MethodPost, "/", h)) + require.NoError(t, onlyError(r.Tree().Handle(http.MethodPost, "/", h))) req := httptest.NewRequest(http.MethodPost, "/", nil) req.Header.Set(HeaderAuthorization, "foobar") w := httptest.NewRecorder() @@ -94,10 +94,10 @@ func TestRecoveryMiddlewareWithBrokenPipe(t *testing.T) { http.Error(c.Writer(), http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) } }))) - require.NoError(t, f.Handle(http.MethodGet, "/foo", func(c Context) { + require.NoError(t, onlyError(f.Handle(http.MethodGet, "/foo", func(c Context) { e := &net.OpError{Err: &os.SyscallError{Err: errno}} panic(e) - })) + }))) req := httptest.NewRequest(http.MethodGet, "/foo", nil) w := httptest.NewRecorder() diff --git a/route_test.go b/route_test.go index 0cdb3f6..631a174 100644 --- a/route_test.go +++ b/route_test.go @@ -196,7 +196,7 @@ func TestRoute_HydrateParams(t *testing.T) { func TestRoute_HandleMiddlewareMalloc(t *testing.T) { f := New() for _, rte := range githubAPI { - require.NoError(t, f.Tree().Handle(rte.method, rte.path, emptyHandler)) + require.NoError(t, onlyError(f.Tree().Handle(rte.method, rte.path, emptyHandler))) } for _, rte := range githubAPI { diff --git a/tree.go b/tree.go index fd62253..3774f75 100644 --- a/tree.go +++ b/tree.go @@ -6,6 +6,7 @@ package fox import ( "fmt" + "math" "net/http" "slices" "strings" @@ -18,8 +19,8 @@ import ( // // IMPORTANT: // Each tree as its own sync.Mutex that may be used to serialize write. Since the router tree may be swapped at any -// given time, you MUST always copy the pointer locally to avoid inadvertently causing a deadlock by locking/unlocking -// the wrong Tree. +// given time (see [Router.Swap]), you MUST always copy the pointer locally to avoid inadvertently causing a deadlock +// by locking/unlocking the wrong Tree. // // Good: // t := fox.Tree() @@ -39,52 +40,76 @@ type Tree struct { maxDepth atomic.Uint32 } -// Handle registers a new handler for the given method and path. This function return an error if the route -// is already registered or conflict with another. It's perfectly safe to add a new handler while the tree is in use -// for serving requests. However, this function is NOT thread-safe and should be run serially, along with all other -// Tree APIs that perform write operations. To override an existing route, use Update. -func (t *Tree) Handle(method, path string, handler HandlerFunc, opts ...PathOption) error { +// Handle registers a new handler for the given method and path. On success, it returns the newly registered [Route]. +// If an error occurs, it returns one of the following: +// - [ErrRouteExist]: If the route is already registered. +// - [ErrRouteConflict]: If the route conflicts with another. +// - [ErrInvalidRoute]: If the provided method or path is invalid. +// +// It's safe to add a new handler while the tree is in use for serving requests. However, this function is NOT +// thread-safe and should be run serially, along with all other [Tree] APIs that perform write operations. +// To override an existing route, use [Tree.Update]. +func (t *Tree) Handle(method, path string, handler HandlerFunc, opts ...PathOption) (*Route, error) { if handler == nil { - return fmt.Errorf("%w: nil handler", ErrInvalidRoute) + return nil, fmt.Errorf("%w: nil handler", ErrInvalidRoute) } if matched := regEnLetter.MatchString(method); !matched { - return fmt.Errorf("%w: missing or invalid http method", ErrInvalidRoute) + return nil, fmt.Errorf("%w: missing or invalid http method", ErrInvalidRoute) } p, catchAllKey, n, err := parseRoute(path) if err != nil { - return err + 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 - return t.insert(method, p, catchAllKey, uint32(n), t.newRoute(path, handler, opts...)) + if err = t.insert(method, p, catchAllKey, uint32(n), rte); err != nil { + return nil, err + } + return rte, nil } -// Update override an existing handler for the given method and path. If the route does not exist, -// the function return an ErrRouteNotFound. It's perfectly safe to update a handler while the tree is in use for -// serving requests. However, this function is NOT thread-safe and should be run serially, along with all other -// Tree APIs that perform write operations. To add a new handler, use Handle method. -func (t *Tree) Update(method, path string, handler HandlerFunc, opts ...PathOption) error { +// Update override an existing handler for the given method and path. On success, it returns the newly registered [Route]. +// If an error occurs, it returns one of the following: +// - [ErrRouteNotFound]: if the route does not exist. +// - [ErrInvalidRoute]: If the provided method or path is invalid. +// +// It's safe to update a handler while the tree is in use for serving requests. However, this function is NOT thread-safe +// and should be run serially, along with all other [Tree] APIs that perform write operations. To add a new handler, +// use [Tree.Handle] method. +func (t *Tree) Update(method, path string, handler HandlerFunc, opts ...PathOption) (*Route, error) { if handler == nil { - return fmt.Errorf("%w: nil handler", ErrInvalidRoute) + return nil, fmt.Errorf("%w: nil handler", ErrInvalidRoute) } if method == "" { - return fmt.Errorf("%w: missing http method", ErrInvalidRoute) + return nil, fmt.Errorf("%w: missing http method", ErrInvalidRoute) } p, catchAllKey, _, err := parseRoute(path) if err != nil { - return err + return nil, err + } + + rte := t.newRoute(path, handler, opts...) + if err = t.update(method, p, catchAllKey, rte); err != nil { + return nil, err } - return t.update(method, p, catchAllKey, t.newRoute(path, handler, opts...)) + return rte, nil } -// Remove delete an existing handler for the given method and path. If the route does not exist, the function -// return an ErrRouteNotFound. It's perfectly safe to remove a handler while the tree is in use for serving requests. -// However, this function is NOT thread-safe and should be run serially, along with all other Tree APIs that perform -// write operations. -func (t *Tree) Remove(method, path string) error { +// Delete deletes an existing handler for the given method and path. If an error occurs, it returns one of the following: +// - [ErrRouteNotFound]: if the route does not exist. +// - [ErrInvalidRoute]: If the provided method or path is invalid. +// It's safe to delete a handler while the tree is in use for serving requests. However, this function is NOT +// thread-safe and should be run serially, along with all other [Tree] APIs that perform write operations. +func (t *Tree) Delete(method, path string) error { if method == "" { return fmt.Errorf("%w: missing http method", ErrInvalidRoute) } @@ -102,15 +127,15 @@ func (t *Tree) Remove(method, path string) error { } // Has allows to check if the given method and path exactly match a registered route. This function is safe for -// concurrent use by multiple goroutine and while mutation on Tree are ongoing. +// concurrent use by multiple goroutine and while mutation on [Tree] are ongoing. See also [Tree.Route] as an alternative. // This API is EXPERIMENTAL and is likely to change in future release. func (t *Tree) Has(method, path string) bool { return t.Route(method, path) != nil } -// Route performs a lookup for a registered route matching the given method and path. It returns the route if a +// Route performs a lookup for a registered route matching the given method and path. It returns the [Route] if a // match is found or nil otherwise. This function is safe for concurrent use by multiple goroutine and while -// mutation on Tree are ongoing. +// mutation on [Tree] are ongoing. See also [Tree.Has] as an alternative. // This API is EXPERIMENTAL and is likely to change in future release. func (t *Tree) Route(method, path string) *Route { nds := *t.nodes.Load() @@ -129,10 +154,10 @@ func (t *Tree) Route(method, path string) *Route { return nil } -// Reverse 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 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. +// 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) Reverse(method, path string) (route *Route, tsr bool) { nds := *t.nodes.Load() @@ -151,12 +176,11 @@ func (t *Tree) Reverse(method, path string) (route *Route, tsr bool) { return nil, false } -// 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 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. +// 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 recommended). If there is a direct match or a tsr is possible, Lookup always return a +// [Route] and a [ContextCloser]. The [ContextCloser] should always be closed if non-nil. This function is safe for +// concurrent use by multiple goroutine and while mutation on [Tree] are ongoing. See also [Tree.Reverse] as an alternative. // This API is EXPERIMENTAL and is likely to change in future release. func (t *Tree) Lookup(w ResponseWriter, r *http.Request) (route *Route, cc ContextCloser, tsr bool) { nds := *t.nodes.Load() @@ -186,7 +210,7 @@ func (t *Tree) Lookup(w ResponseWriter, r *http.Request) (route *Route, cc Conte } // 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 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 (t *Tree) Iter() Iter { return Iter{t: t}