diff --git a/README.md b/README.md index 9e531a3..60c81af 100644 --- a/README.md +++ b/README.md @@ -91,39 +91,57 @@ if errors.Is(err, fox.ErrRouteConflict) { ``` #### Named parameters -A route can be defined using placeholder (e.g `{name}`). The matching segment are recorder into `fox.Param` accessible -via `fox.Context`. `fox.Context.Params` provide an iterator to range over `fox.Param` and `fox.Context.Param` allow -to retrieve directly the value of a parameter using the placeholder name. +Routes can include named parameters using curly braces `{}` to match exactly one non-empty path segment. The matching +segment are recorder into `fox.Param` accessible via `fox.Context`. `fox.Context.Params` provide an iterator to range +over `fox.Param` and `fox.Context.Param` allow to retrieve directly the value of a parameter using the placeholder name. ```` Pattern /avengers/{name} -/avengers/ironman match -/avengers/thor match -/avengers/hulk/angry no match -/avengers/ no match +/avengers/ironman matches +/avengers/thor matches +/avengers/hulk/angry no matches +/avengers/ no matches Pattern /users/uuid:{id} -/users/uuid:123 match -/users/uuid no match +/users/uuid:123 matches +/users/uuid no matches ```` #### Catch all parameter -Catch-all parameters can be used to match everything at the end of a route. The placeholder start with `*` followed by a regular -named parameter (e.g. `*{name}`). +Catch-all parameters start with an asterisk `*` followed by a name `{param}` and match one or more **non-empty** path segments, +including slashes. They can be placed anywhere in the route path but **cannot be consecutive**. The matching segment are also +accessible via `fox.Context` + +**Example with ending catch all** ```` Pattern /src/*{filepath} -/src/ match -/src/conf.txt match -/src/dir/config.txt match +/src/conf.txt matches +/src/dir/config.txt matches +/src/ no matches + +Pattern /src/file=*{path} + +/src/file=config.txt matches +/src/file=/dir/config.txt matches +/src/file= no matches +```` + +**Example with infix catch all** +```` +Pattern: /assets/*{path}/thumbnail + +/assets/images/thumbnail matches +/assets/photos/2021/thumbnail matches +/assets/thumbnail no matches -Patter /src/file=*{path} +Pattern: /assets/path:*{path}/thumbnail -/src/file= match -/src/file=config.txt match -/src/file=/dir/config.txt match +/assets/path:images/thumbnail matches +/assets/path:photos/2021/thumbnail matches +/assets/path:thumbnail no matches ```` #### Priority rules @@ -151,9 +169,17 @@ POST /users/{name}/emails Additionally, let's consider an example to illustrate the prioritization: ```` -GET /fs/avengers.txt #1 => match /fs/avengers.txt -GET /fs/{filename} #2 => match /fs/ironman.txt -GET /fs/*{filepath} #3 => match /fs/avengers/ironman.txt +Route Definitions: + +1. GET /fs/avengers.txt # Highest priority (static) +2. GET /fs/{filename} # Next priority (named parameter) +3. GET /fs/*{filepath} # Lowest priority (catch-all parameter) + +Request Matching: + +- /fs/avengers.txt matches Route 1 +- /fs/ironman.txt matches Route 2 +- /fs/avengers/ironman.txt matches Route 3 ```` #### Warning about context diff --git a/context.go b/context.go index 0f0119e..881e296 100644 --- a/context.go +++ b/context.go @@ -402,17 +402,6 @@ func (c *cTx) CloneWith(w ResponseWriter, r *http.Request) ContextCloser { return cp } -func copyParams(src, dst *Params) { - if cap(*src) > cap(*dst) { - // Grow dst to a least cap(src) - *dst = slices.Grow(*dst, cap(*src)) - } - // cap(dst) >= cap(src) - // now constraint into len(src) & cap(src) - *dst = (*dst)[:len(*src):cap(*src)] - copy(*dst, *src) -} - // Scope returns the HandlerScope associated with the current Context. // This indicates the scope in which the handler is being executed, such as RouteHandler, NoRouteHandler, etc. func (c *cTx) Scope() HandlerScope { diff --git a/context_test.go b/context_test.go index f3946c5..91f497e 100644 --- a/context_test.go +++ b/context_test.go @@ -210,11 +210,11 @@ func TestContext_Annotations(t *testing.T) { "/foo", emptyHandler, WithAnnotations(Annotation{Key: "foo", Value: "bar"}, Annotation{Key: "foo", Value: "baz"}), - WithAnnotation("john", 1), + WithAnnotations(Annotation{Key: "john", Value: 1}), ) rte := f.Tree().Route(http.MethodGet, "/foo") require.NotNil(t, rte) - assert.Equal(t, []Annotation{{"foo", "bar"}, {"foo", "baz"}, {"john", 1}}, slices.Collect(rte.Annotations())) + assert.Equal(t, []Annotation{{Key: "foo", Value: "bar"}, {Key: "foo", Value: "baz"}, {Key: "john", Value: 1}}, slices.Collect(rte.Annotations())) } func TestContext_Clone(t *testing.T) { diff --git a/error.go b/error.go index 6341501..28adcee 100644 --- a/error.go +++ b/error.go @@ -32,10 +32,7 @@ type RouteConflictError struct { isUpdate bool } -func newConflictErr(method, path, catchAllKey string, matched []string) *RouteConflictError { - if catchAllKey != "" { - path += "*{" + catchAllKey + "}" - } +func newConflictErr(method, path string, matched []string) *RouteConflictError { return &RouteConflictError{ Method: method, Path: path, diff --git a/fox.go b/fox.go index 6493201..b6af9bf 100644 --- a/fox.go +++ b/fox.go @@ -6,6 +6,7 @@ package fox import ( "fmt" + "math" "net" "net/http" "path" @@ -343,11 +344,11 @@ func (fox *Router) ServeHTTP(w http.ResponseWriter, r *http.Request) { nds := *tree.nodes.Load() index := findRootNode(r.Method, nds) - if index < 0 { + if index < 0 || len(nds[index].children) == 0 { goto NoMethodFallback } - n, tsr = tree.lookup(nds[index], target, c, false) + n, tsr = tree.lookup(nds[index].children[0].Load(), target, c, false) if !tsr && n != nil { c.route = n.route c.tsr = tsr @@ -404,11 +405,13 @@ NoMethodFallback: } else { // Since different method and route may match (e.g. GET /foo/bar & POST /foo/{name}), we cannot set the path and params. for i := 0; i < len(nds); i++ { - if n, tsr := tree.lookup(nds[i], target, c, true); n != nil && (!tsr || n.route.ignoreTrailingSlash) { - if sb.Len() > 0 { - sb.WriteString(", ") + if len(nds[i].children) > 0 { + if n, tsr := tree.lookup(nds[i].children[0].Load(), target, c, true); n != nil && (!tsr || n.route.ignoreTrailingSlash) { + if sb.Len() > 0 { + sb.WriteString(", ") + } + sb.WriteString(nds[i].key) } - sb.WriteString(nds[i].key) } } } @@ -425,15 +428,18 @@ NoMethodFallback: var sb strings.Builder for i := 0; i < len(nds); i++ { if nds[i].key != r.Method { - if n, tsr := tree.lookup(nds[i], target, c, true); n != nil && (!tsr || n.route.ignoreTrailingSlash) { - if sb.Len() > 0 { - sb.WriteString(", ") + if len(nds[i].children) > 0 { + if n, tsr := tree.lookup(nds[i].children[0].Load(), target, c, true); n != nil && (!tsr || n.route.ignoreTrailingSlash) { + if sb.Len() > 0 { + sb.WriteString(", ") + } + sb.WriteString(nds[i].key) } - sb.WriteString(nds[i].key) } } } if sb.Len() > 0 { + // TODO maybe should add OPTIONS ? w.Header().Set(HeaderAllow, sb.String()) c.scope = NoMethodHandler fox.noMethod(c) @@ -536,16 +542,16 @@ const ( ) // parseRoute parse and validate the route in a single pass. -func parseRoute(path string) (string, string, int, error) { +func parseRoute(path string) (uint32, error) { if !strings.HasPrefix(path, "/") { - return "", "", -1, fmt.Errorf("%w: path must start with '/'", ErrInvalidRoute) + return 0, fmt.Errorf("%w: path must start with '/'", ErrInvalidRoute) } state := stateDefault previous := stateDefault - startCatchAll := 0 - paramCnt := 0 + paramCnt := uint32(0) + countStatic := 0 inParam := false i := 0 @@ -554,75 +560,87 @@ func parseRoute(path string) (string, string, int, error) { case stateParam: if path[i] == '}' { if !inParam { - return "", "", -1, fmt.Errorf("%w: missing parameter name between '{}'", ErrInvalidRoute) + return 0, fmt.Errorf("%w: missing parameter name between '{}'", ErrInvalidRoute) } inParam = false - if previous != stateCatchAll { - if i+1 < len(path) && path[i+1] != '/' { - return "", "", -1, fmt.Errorf("%w: unexpected character after '{param}'", ErrInvalidRoute) - } - } else { - if i+1 != len(path) { - return "", "", -1, fmt.Errorf("%w: catch-all '*{params}' are allowed only at the end of a route", ErrInvalidRoute) - } + if i+1 < len(path) && path[i+1] != '/' { + return 0, fmt.Errorf("%w: unexpected character after '{param}'", ErrInvalidRoute) } + + countStatic = 0 + previous = state state = stateDefault i++ continue } if path[i] == '/' || path[i] == '*' || path[i] == '{' { - return "", "", -1, fmt.Errorf("%w: unexpected character in '{params}'", ErrInvalidRoute) + return 0, fmt.Errorf("%w: unexpected character in '{params}'", ErrInvalidRoute) } inParam = true i++ - case stateCatchAll: - if path[i] != '{' { - return "", "", -1, fmt.Errorf("%w: unexpected character after '*' catch-all delimiter", ErrInvalidRoute) + if path[i] == '}' { + if !inParam { + return 0, fmt.Errorf("%w: missing parameter name between '*{}'", ErrInvalidRoute) + } + inParam = false + if i+1 < len(path) && path[i+1] != '/' { + return 0, fmt.Errorf("%w: unexpected character after '*{param}'", ErrInvalidRoute) + } + + if previous == stateCatchAll && countStatic <= 1 { + return 0, fmt.Errorf("%w: consecutive wildcard not allowed", ErrInvalidRoute) + } + + countStatic = 0 + previous = state + state = stateDefault + i++ + continue } - startCatchAll = i - previous = state - state = stateParam - i++ + if path[i] == '/' || path[i] == '*' || path[i] == '{' { + return 0, fmt.Errorf("%w: unexpected character in '*{params}'", ErrInvalidRoute) + } + inParam = true + i++ default: if path[i] == '{' { state = stateParam paramCnt++ } else if path[i] == '*' { state = stateCatchAll + i++ paramCnt++ + } else { + countStatic++ + } + + if paramCnt > math.MaxUint16 { + return 0, fmt.Errorf("%w: too many params (%d)", ErrInvalidRoute, paramCnt) } + i++ } } if state == stateParam { - return "", "", -1, fmt.Errorf("%w: unclosed '{params}'", ErrInvalidRoute) - } - if state == stateCatchAll { - return "", "", -1, fmt.Errorf("%w: missing '{params}' after '*' catch-all delimiter", ErrInvalidRoute) + return 0, fmt.Errorf("%w: unclosed '{params}'", ErrInvalidRoute) } - if startCatchAll > 0 { - return path[:startCatchAll-1], path[startCatchAll+1 : len(path)-1], paramCnt, nil + if state == stateCatchAll { + if path[len(path)-1] == '*' { + return 0, fmt.Errorf("%w: missing '{params}' after '*' catch-all delimiter", ErrInvalidRoute) + } + return 0, fmt.Errorf("%w: unclosed '*{params}'", ErrInvalidRoute) } - return path, "", paramCnt, nil + return paramCnt, nil } func getRouteConflict(n *node) []string { routes := make([]string, 0) - - if n.isCatchAll() { - routes = append(routes, n.route.path) - return routes - } - - if n.paramChildIndex >= 0 { - n = n.children[n.paramChildIndex].Load() - } it := newRawIterator(n) for it.hasNext() { routes = append(routes, it.current.route.path) diff --git a/fox_test.go b/fox_test.go index e9e9283..62b2f64 100644 --- a/fox_test.go +++ b/fox_test.go @@ -509,6 +509,21 @@ func BenchmarkGithubParamsAll(b *testing.B) { } } +func BenchmarkInfixCatchAll(b *testing.B) { + f := New() + f.MustHandle(http.MethodGet, "/foo/*{bar}/baz", emptyHandler) + + req := httptest.NewRequest(http.MethodGet, "/foo/a1/b22/c333/baz", nil) + w := new(mockResponseWriter) + + b.ReportAllocs() + b.ResetTimer() + + for i := 0; i < b.N; i++ { + f.ServeHTTP(w, req) + } +} + func BenchmarkLongParam(b *testing.B) { r := New() r.MustHandle(http.MethodGet, "/foo/{very_very_very_very_very_long_param}", emptyHandler) @@ -708,7 +723,7 @@ func TestRouterWildcard(t *testing.T) { }{ {"/github.com/etf1/*{repo}", "/github.com/etf1/mux"}, {"/github.com/johndoe/*{repo}", "/github.com/johndoe/buzz"}, - {"/foo/bar/*{args}", "/foo/bar/"}, + {"/foo/bar/*{args}", "/foo/bar/baz"}, {"/filepath/path=*{path}", "/filepath/path=/file.txt"}, } @@ -720,11 +735,108 @@ func TestRouterWildcard(t *testing.T) { req := httptest.NewRequest(http.MethodGet, route.key, nil) w := httptest.NewRecorder() r.ServeHTTP(w, req) - require.Equalf(t, http.StatusOK, w.Code, "route: key: %s, path: %s", route.path) + require.Equalf(t, http.StatusOK, w.Code, "route: key: %s, path: %s", route.key, route.path) assert.Equal(t, route.key, w.Body.String()) } } +func TestEmptyCatchAll(t *testing.T) { + + cases := []struct { + name string + routes []string + path string + }{ + { + name: "infix wildcard", + routes: []string{"/foo/*{args}/bar"}, + path: "/foo/bar", + }, + { + name: "infix wildcard with children", + routes: []string{"/foo/*{args}/bar", "/foo/*{args}/caz"}, + path: "/foo/bar", + }, + { + name: "infix wildcard with static edge", + routes: []string{"/foo/*{args}/bar", "/foo/baz"}, + path: "/foo/bar", + }, + { + name: "infix wildcard and suffix wildcard", + routes: []string{"/foo/*{args}/bar", "/foo/*{args}"}, + path: "/foo/", + }, + // + { + name: "infix inflight wildcard", + routes: []string{"/foo/abc*{args}/bar"}, + path: "/foo/abc/bar", + }, + { + name: "infix inflight wildcard with children", + routes: []string{"/foo/abc*{args}/bar", "/foo/abc*{args}/caz"}, + path: "/foo/abc/bar", + }, + { + name: "infix inflight wildcard with static edge", + routes: []string{"/foo/abc*{args}/bar", "/foo/abc/baz"}, + path: "/foo/abc/bar", + }, + { + name: "infix inflight wildcard and suffix wildcard", + routes: []string{"/foo/abc*{args}/bar", "/foo/abc*{args}"}, + path: "/foo/abc", + }, + { + name: "suffix wildcard wildcard with param edge", + routes: []string{"/foo/*{args}", "/foo/{param}"}, + path: "/foo/", + }, + { + name: "suffix inflight wildcard wildcard with param edge", + routes: []string{"/foo/abc*{args}", "/foo/abc{param}"}, + path: "/foo/abc", + }, + { + name: "infix wildcard wildcard with param edge", + routes: []string{"/foo/*{args}/bar", "/foo/{param}/bar"}, + path: "/foo/bar", + }, + { + name: "infix inflight wildcard wildcard with param edge", + routes: []string{"/foo/abc*{args}/bar", "/foo/abc{param}/bar"}, + path: "/foo/abc/bar", + }, + { + name: "infix wildcard wildcard with trailing slash", + routes: []string{"/foo/*{args}/"}, + path: "/foo//", + }, + { + name: "infix inflight wildcard wildcard with trailing slash", + routes: []string{"/foo/abc*{args}/"}, + path: "/foo/abc/", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + f := New() + tree := f.Tree() + for _, rte := range tc.routes { + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + } + nds := *tree.nodes.Load() + c := newTestContextTree(tree) + n, tsr := tree.lookup(nds[0].children[0].Load(), tc.path, c, false) + require.False(t, tsr) + require.Nil(t, n) + }) + } + +} + func TestRouteWithParams(t *testing.T) { tree := New().Tree() routes := [...]string{ @@ -750,9 +862,9 @@ func TestRouteWithParams(t *testing.T) { nds := *tree.nodes.Load() for _, rte := range routes { c := newTestContextTree(tree) - n, tsr := tree.lookup(nds[0], rte, c, false) - require.NotNil(t, n) - require.NotNil(t, n.route) + n, tsr := tree.lookup(nds[0].children[0].Load(), rte, c, false) + require.NotNilf(t, n, "route: %s", rte) + require.NotNilf(t, n.route, "route: %s", rte) assert.False(t, tsr) assert.Equal(t, rte, n.route.path) } @@ -790,7 +902,7 @@ func TestRouteParamEmptySegment(t *testing.T) { t.Run(tc.name, func(t *testing.T) { nds := *tree.nodes.Load() c := newTestContextTree(tree) - n, tsr := tree.lookup(nds[0], tc.path, c, false) + n, tsr := tree.lookup(nds[0].children[0].Load(), tc.path, c, false) assert.Nil(t, n) assert.Empty(t, slices.Collect(c.Params())) assert.False(t, tsr) @@ -1168,7 +1280,7 @@ func TestOverlappingRoute(t *testing.T) { }, { name: "backtrack to most specific catch-all with an exact match", - path: "/foo/bar/", + path: "/foo/bar/x/y/z", routes: []string{ "/foo/{id_1}/abc", "/foo/{id_1}/*{args}", @@ -1182,7 +1294,25 @@ func TestOverlappingRoute(t *testing.T) { Value: "bar", }, { - Key: "args", + Key: "args", + Value: "x/y/z", + }, + }, + }, + { + name: "backtrack to most specific catch-all with an exact match", + path: "/foo/bar/", + routes: []string{ + "/foo/{id_1}/abc", + "/foo/{id_1}/*{args}", + "/foo/{id_1}/{id_2}", + "/foo/*{args}", + }, + wantMatch: "/foo/*{args}", + wantParams: Params{ + { + Key: "args", + Value: "bar/", }, }, }, @@ -1244,7 +1374,7 @@ func TestOverlappingRoute(t *testing.T) { nds := *tree.nodes.Load() c := newTestContextTree(tree) - n, tsr := tree.lookup(nds[0], tc.path, c, false) + n, tsr := tree.lookup(nds[0].children[0].Load(), tc.path, c, false) require.NotNil(t, n) require.NotNil(t, n.route) assert.False(t, tsr) @@ -1258,7 +1388,7 @@ func TestOverlappingRoute(t *testing.T) { // Test with lazy c = newTestContextTree(tree) - n, tsr = tree.lookup(nds[0], tc.path, c, true) + n, tsr = tree.lookup(nds[0].children[0].Load(), tc.path, c, true) require.NotNil(t, n) require.NotNil(t, n.route) assert.False(t, tsr) @@ -1268,169 +1398,1229 @@ func TestOverlappingRoute(t *testing.T) { } } -func TestInsertConflict(t *testing.T) { +func TestInfixWildcard(t *testing.T) { cases := []struct { - name string - routes []struct { - wantErr error - path string - wantMatch []string - } + name string + routes []string + path string + wantPath string + wantTsr bool + wantParams []Param }{ { - name: "exact match conflict", - routes: []struct { - wantErr error - path string - wantMatch []string - }{ - {path: "/john/*{x}", wantErr: nil, wantMatch: nil}, - {path: "/john/*{y}", wantErr: ErrRouteConflict, wantMatch: []string{"/john/*{x}"}}, - {path: "/john/", wantErr: ErrRouteExist, wantMatch: nil}, - {path: "/foo/baz", wantErr: nil, wantMatch: nil}, - {path: "/foo/bar", wantErr: nil, wantMatch: nil}, - {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, - {path: "/foo/*{args}", wantErr: nil, wantMatch: nil}, - {path: "/avengers/ironman/{power}", wantErr: nil, wantMatch: nil}, - {path: "/avengers/{id}/bar", wantErr: nil, wantMatch: nil}, - {path: "/avengers/{id}/foo", wantErr: nil, wantMatch: nil}, - {path: "/avengers/*{args}", wantErr: nil, wantMatch: nil}, - {path: "/fox/", wantErr: nil, wantMatch: nil}, - {path: "/fox/*{args}", wantErr: ErrRouteExist, wantMatch: nil}, + name: "simple infix wildcard", + routes: []string{"/foo/*{args}/bar"}, + path: "/foo/a/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a", + }, }, }, { - name: "no conflict for incomplete match to end of edge", - routes: []struct { - wantErr error - path string - wantMatch []string - }{ - {path: "/foo/bar", wantErr: nil, wantMatch: nil}, - {path: "/foo/baz", wantErr: nil, wantMatch: nil}, - {path: "/foo/*{args}", wantErr: nil, wantMatch: nil}, - {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, + name: "simple infix wildcard", + routes: []string{"/foo/*{args}/bar"}, + path: "/foo/a/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a", + }, }, }, { - name: "no conflict for key match mid-edge", - routes: []struct { - wantErr error - path string - wantMatch []string - }{ - {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, - {path: "/foo/*{args}", wantErr: nil, wantMatch: nil}, - {path: "/foo/a*{args}", wantErr: nil, wantMatch: nil}, - {path: "/foo*{args}", wantErr: nil, wantMatch: nil}, - {path: "/john{doe}", wantErr: nil, wantMatch: nil}, - {path: "/john*{doe}", wantErr: nil, wantMatch: nil}, - {path: "/john/{doe}", wantErr: nil, wantMatch: nil}, - {path: "/joh{doe}", wantErr: nil, wantMatch: nil}, - {path: "/avengers/{id}/foo", wantErr: nil, wantMatch: nil}, - {path: "/avengers/{id}/bar", wantErr: nil, wantMatch: nil}, - {path: "/avengers/*{args}", wantErr: nil, wantMatch: nil}, + name: "static with infix wildcard child", + routes: []string{"/foo/", "/foo/*{args}/baz"}, + path: "/foo/bar/baz", + wantPath: "/foo/*{args}/baz", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "bar", + }, }, }, { - name: "incomplete match to middle of edge", - routes: []struct { - wantErr error - path string - wantMatch []string - }{ - {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, - {path: "/foo/{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/foo/{id}"}}, - {path: "/foo{id}", wantErr: nil, wantMatch: nil}, - {path: "/foo/a{id}", wantErr: nil, wantMatch: nil}, - {path: "/avengers/{id}/bar", wantErr: nil, wantMatch: nil}, - {path: "/avengers/{id}/baz", wantErr: nil, wantMatch: nil}, - {path: "/avengers/{id}", wantErr: nil, wantMatch: nil}, - {path: "/avengers/{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/avengers/{id}", "/avengers/{id}/bar", "/avengers/{id}/baz"}}, + name: "simple infix wildcard with route char", + routes: []string{"/foo/*{args}/bar"}, + path: "/foo/*{args}/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "*{args}", + }, }, }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - tree := New().Tree() - for _, rte := range tc.routes { - r, err := tree.Handle(http.MethodGet, rte.path, emptyHandler) - if err != nil { - assert.Nil(t, r) - } - assert.ErrorIs(t, err, rte.wantErr) - if cErr, ok := err.(*RouteConflictError); ok { - assert.Equal(t, rte.wantMatch, cErr.Matched) - } - } - }) - } -} - -func TestUpdateConflict(t *testing.T) { - cases := []struct { - name string - routes []string - update string - wantErr error - wantMatch []string - }{ { - name: "wildcard parameter route not registered", - routes: []string{"/foo/{bar}"}, - update: "/foo/{baz}", - wantErr: ErrRouteNotFound, + name: "simple infix wildcard with multi segment and route char", + routes: []string{"/foo/*{args}/bar"}, + path: "/foo/*{args}/b/c/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "*{args}/b/c", + }, + }, }, { - name: "wildcard catch all route not registered", - routes: []string{"/foo/{bar}"}, - update: "/foo/*{baz}", - wantErr: ErrRouteNotFound, + name: "simple infix inflight wildcard", + routes: []string{"/foo/z*{args}/bar"}, + path: "/foo/za/bar", + wantPath: "/foo/z*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a", + }, + }, }, { - name: "route match but not a leaf", - routes: []string{"/foo/bar/baz"}, - update: "/foo/bar", - wantErr: ErrRouteNotFound, + name: "simple infix inflight wildcard with route char", + routes: []string{"/foo/z*{args}/bar"}, + path: "/foo/z*{args}/bar", + wantPath: "/foo/z*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "*{args}", + }, + }, }, { - name: "wildcard have different name", - routes: []string{"/foo/bar", "/foo/*{args}"}, - update: "/foo/*{all}", - wantErr: ErrRouteConflict, - wantMatch: []string{"/foo/*{args}"}, + name: "simple infix inflight wildcard with multi segment", + routes: []string{"/foo/z*{args}/bar"}, + path: "/foo/za/b/c/bar", + wantPath: "/foo/z*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c", + }, + }, }, { - name: "replacing non wildcard by wildcard", - routes: []string{"/foo/bar", "/foo/"}, - update: "/foo/*{all}", - wantErr: ErrRouteConflict, - wantMatch: []string{"/foo/"}, + name: "simple infix inflight wildcard with multi segment and route char", + routes: []string{"/foo/z*{args}/bar"}, + path: "/foo/z*{args}/b/c/bar", + wantPath: "/foo/z*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "*{args}/b/c", + }, + }, }, { - name: "replacing wildcard by non wildcard", - routes: []string{"/foo/bar", "/foo/*{args}"}, - update: "/foo/", - wantErr: ErrRouteConflict, - wantMatch: []string{"/foo/*{args}"}, + name: "simple infix inflight wildcard long", + routes: []string{"/foo/xyz*{args}/bar"}, + path: "/foo/xyza/bar", + wantPath: "/foo/xyz*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a", + }, + }, }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - tree := New().Tree() - for _, rte := range tc.routes { - require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) - } - r, err := tree.Update(http.MethodGet, tc.update, emptyHandler) - if err != nil { - assert.Nil(t, r) - } - assert.ErrorIs(t, err, tc.wantErr) - if cErr, ok := err.(*RouteConflictError); ok { - assert.Equal(t, tc.wantMatch, cErr.Matched) + { + name: "simple infix inflight wildcard with multi segment long", + routes: []string{"/foo/xyz*{args}/bar"}, + path: "/foo/xyza/b/c/bar", + wantPath: "/foo/xyz*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c", + }, + }, + }, + { + name: "overlapping infix and suffix wildcard match infix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar"}, + path: "/foo/a/b/c/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c", + }, + }, + }, + { + name: "overlapping infix and suffix wildcard match suffix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar"}, + path: "/foo/a/b/c/baz", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c/baz", + }, + }, + }, + { + name: "overlapping infix and suffix wildcard match suffix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar"}, + path: "/foo/a/b/c/barito", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c/barito", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match infix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/bar"}, + path: "/foo/a/b/c/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match suffix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/bar"}, + path: "/foo/a/b/c/bili", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/b/c/bili", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match infix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}"}, + path: "/foo/a/bar", + wantPath: "/foo/*{args}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match param", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/bar"}, + path: "/foo/a/bar", + wantPath: "/foo/{ps}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "a", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match suffix", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}"}, + path: "/foo/a/bili", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/bili", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match param", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}"}, + path: "/foo/a", + wantPath: "/foo/{ps}", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "a", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match param with ts", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/"}, + path: "/foo/a/", + wantPath: "/foo/{ps}/", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "a", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match suffix without ts", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}/"}, + path: "/foo/a", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a", + }, + }, + }, + { + name: "overlapping infix suffix wildcard and param match suffix without ts", + routes: []string{"/foo/*{args}", "/foo/*{args}/bar", "/foo/{ps}"}, + path: "/foo/a/", + wantPath: "/foo/*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/", + }, + }, + }, + { + name: "overlapping infix inflight suffix wildcard and param match param", + routes: []string{"/foo/123*{args}", "/foo/123*{args}/bar", "/foo/123{ps}/bar"}, + path: "/foo/123a/bar", + wantPath: "/foo/123{ps}/bar", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "a", + }, + }, + }, + { + name: "overlapping infix inflight suffix wildcard and param match suffix", + routes: []string{"/foo/123*{args}", "/foo/123*{args}/bar", "/foo/123{ps}"}, + path: "/foo/123a/bili", + wantPath: "/foo/123*{args}", + wantTsr: false, + wantParams: Params{ + { + Key: "args", + Value: "a/bili", + }, + }, + }, + { + name: "overlapping infix inflight suffix wildcard and param match param", + routes: []string{"/foo/123*{args}", "/foo/123*{args}/bar", "/foo/123{ps}"}, + path: "/foo/123a", + wantPath: "/foo/123{ps}", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "a", + }, + }, + }, + { + name: "infix segment followed by param", + routes: []string{"/foo/*{a}/{b}"}, + path: "/foo/a/b/c/d", + wantPath: "/foo/*{a}/{b}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a/b/c", + }, + { + Key: "b", + Value: "d", + }, + }, + }, + { + name: "infix segment followed by two params", + routes: []string{"/foo/*{a}/{b}/{c}"}, + path: "/foo/a/b/c/d", + wantPath: "/foo/*{a}/{b}/{c}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a/b", + }, + { + Key: "b", + Value: "c", + }, + { + Key: "c", + Value: "d", + }, + }, + }, + { + name: "infix segment followed by one param and one wildcard", + routes: []string{"/foo/*{a}/{b}/*{c}"}, + path: "/foo/a/b/c/d", + wantPath: "/foo/*{a}/{b}/*{c}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a", + }, + { + Key: "b", + Value: "b", + }, + { + Key: "c", + Value: "c/d", + }, + }, + }, + { + name: "param followed by suffix wildcard", + routes: []string{"/foo/{a}/*{b}"}, + path: "/foo/a/b/c/d", + wantPath: "/foo/{a}/*{b}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a", + }, + { + Key: "b", + Value: "b/c/d", + }, + }, + }, + { + name: "infix inflight segment followed by param", + routes: []string{"/foo/123*{a}/{b}"}, + path: "/foo/123a/b/c/d", + wantPath: "/foo/123*{a}/{b}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a/b/c", + }, + { + Key: "b", + Value: "d", + }, + }, + }, + { + name: "inflight param followed by suffix wildcard", + routes: []string{"/foo/123{a}/*{b}"}, + path: "/foo/123a/b/c/d", + wantPath: "/foo/123{a}/*{b}", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "a", + }, + { + Key: "b", + Value: "b/c/d", + }, + }, + }, + { + name: "multi infix segment simple", + routes: []string{"/foo/*{$1}/bar/*{$2}/baz"}, + path: "/foo/a/bar/b/c/d/baz", + wantPath: "/foo/*{$1}/bar/*{$2}/baz", + wantTsr: false, + wantParams: Params{ + { + Key: "$1", + Value: "a", + }, + { + Key: "$2", + Value: "b/c/d", + }, + }, + }, + { + name: "multi inflight segment simple", + routes: []string{"/foo/123*{$1}/bar/456*{$2}/baz"}, + path: "/foo/123a/bar/456b/c/d/baz", + wantPath: "/foo/123*{$1}/bar/456*{$2}/baz", + wantTsr: false, + wantParams: Params{ + { + Key: "$1", + Value: "a", + }, + { + Key: "$2", + Value: "b/c/d", + }, + }, + }, + { + name: "static priority", + routes: []string{"/foo/bar/baz", "/foo/{ps}/baz", "/foo/*{any}/baz"}, + path: "/foo/bar/baz", + wantPath: "/foo/bar/baz", + wantTsr: false, + }, + { + name: "param priority", + routes: []string{"/foo/bar/baz", "/foo/{ps}/baz", "/foo/*{any}/baz"}, + path: "/foo/buzz/baz", + wantPath: "/foo/{ps}/baz", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "buzz", + }, + }, + }, + { + name: "fallback catch all", + routes: []string{"/foo/bar/baz", "/foo/{ps}/baz", "/foo/*{any}/baz"}, + path: "/foo/a/b/baz", + wantPath: "/foo/*{any}/baz", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b", + }, + }, + }, + { + name: "complex overlapping route with static priority", + routes: []string{ + "/foo/bar/baz/{$1}/jo", + "/foo/*{any}/baz/{$1}/jo", + "/foo/{ps}/baz/{$1}/jo", + }, + path: "/foo/bar/baz/1/jo", + wantPath: "/foo/bar/baz/{$1}/jo", + wantTsr: false, + wantParams: Params{ + { + Key: "$1", + Value: "1", + }, + }, + }, + { + name: "complex overlapping route with param priority", + routes: []string{ + "/foo/bar/baz/{$1}/jo", + "/foo/*{any}/baz/{$1}/jo", + "/foo/{ps}/baz/{$1}/jo", + }, + path: "/foo/bam/baz/1/jo", + wantPath: "/foo/{ps}/baz/{$1}/jo", + wantTsr: false, + wantParams: Params{ + { + Key: "ps", + Value: "bam", + }, + { + Key: "$1", + Value: "1", + }, + }, + }, + { + name: "complex overlapping route with catch all fallback", + routes: []string{ + "/foo/bar/baz/{$1}/jo", + "/foo/*{any}/baz/{$1}/jo", + "/foo/{ps}/baz/{$1}/jo", + }, + path: "/foo/a/b/c/baz/1/jo", + wantPath: "/foo/*{any}/baz/{$1}/jo", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + { + Key: "$1", + Value: "1", + }, + }, + }, + { + name: "complex overlapping route with catch all fallback", + routes: []string{ + "/foo/bar/baz/{$1}/jo", + "/foo/*{any}/baz/{$1}/john", + "/foo/{ps}/baz/{$1}/johnny", + }, + path: "/foo/a/baz/1/john", + wantPath: "/foo/*{any}/baz/{$1}/john", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a", + }, + { + Key: "$1", + Value: "1", + }, + }, + }, + { + name: "overlapping static and infix", + routes: []string{ + "/foo/*{any}/baz", + "/foo/a/b/baz", + }, + path: "/foo/a/b/baz", + wantPath: "/foo/a/b/baz", + wantTsr: false, + }, + { + name: "overlapping static and infix with catch all fallback", + routes: []string{ + "/foo/*{any}/baz", + "/foo/a/b/baz", + }, + path: "/foo/a/b/c/baz", + wantPath: "/foo/*{any}/baz", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, + }, + { + name: "infix wildcard with trailing slash", + routes: []string{ + "/foo/*{any}/", + }, + path: "/foo/a/b/c/", + wantPath: "/foo/*{any}/", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, + }, + { + name: "overlapping static and infix with most specific", + routes: []string{ + "/foo/*{any}/{a}/ddd/", + "/foo/*{any}/bbb/{d}", + }, + path: "/foo/a/b/c/bbb/ddd/", + wantPath: "/foo/*{any}/{a}/ddd/", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + { + Key: "a", + Value: "bbb", + }, + }, + }, + { + name: "infix wildcard with trailing slash", + routes: []string{ + "/foo/*{any}", + "/foo/*{any}/b/", + "/foo/*{any}/c/", + }, + path: "/foo/x/y/z/", + wantPath: "/foo/*{any}", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "x/y/z/", + }, + }, + }, + { + name: "infix wildcard with trailing slash most specific", + routes: []string{ + "/foo/*{any}", + "/foo/*{any}/", + }, + path: "/foo/x/y/z/", + wantPath: "/foo/*{any}/", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "x/y/z", + }, + }, + }, + { + name: "infix wildcard with trailing slash most specific", + routes: []string{ + "/foo/*{any}", + "/foo/*{any}/", + }, + path: "/foo/x/y/z", + wantPath: "/foo/*{any}", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "x/y/z", + }, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + f := New() + tree := f.Tree() + for _, rte := range tc.routes { + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + } + nds := *tree.nodes.Load() + c := newTestContextTree(tree) + n, tsr := tree.lookup(nds[0].children[0].Load(), tc.path, c, false) + require.NotNil(t, n) + assert.Equal(t, tc.wantPath, n.route.path) + assert.Equal(t, tc.wantTsr, tsr) + c.tsr = tsr + assert.Equal(t, tc.wantParams, slices.Collect(c.Params())) + }) + } + +} + +func TestInfixWildcardTsr(t *testing.T) { + cases := []struct { + name string + routes []string + path string + wantPath string + wantTsr bool + wantParams []Param + }{ + { + name: "infix wildcard with trailing slash and tsr add", + routes: []string{ + "/foo/*{any}/", + }, + path: "/foo/a/b/c", + wantPath: "/foo/*{any}/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, + }, + { + name: "infix wildcard with tsr but skipped node match", + routes: []string{ + "/foo/*{any}/", + "/{x}/a/b/c", + }, + path: "/foo/a/b/c", + wantPath: "/{x}/a/b/c", + wantTsr: false, + wantParams: Params{ + { + Key: "x", + Value: "foo", + }, + }, + }, + { + name: "infix wildcard with tsr but skipped node does not match", + routes: []string{ + "/foo/*{any}/", + "/{x}/a/b/x", + }, + path: "/foo/a/b/c", + wantPath: "/foo/*{any}/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, + }, + { + name: "infix wildcard with trailing slash and tsr add", + routes: []string{ + "/foo/*{any}/", + "/foo/*{any}/abc", + "/foo/*{any}/bcd", + }, + path: "/foo/a/b/c/abd", + wantPath: "/foo/*{any}/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c/abd", + }, + }, + }, + { + name: "infix wildcard with sub-node tsr add fallback", + routes: []string{ + "/foo/*{any}/{a}/ddd/", + "/foo/*{any}/bbb/{d}/foo", + }, + path: "/foo/a/b/c/bbb/ddd", + wantPath: "/foo/*{any}/{a}/ddd/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + { + Key: "a", + Value: "bbb", + }, + }, + }, + { + name: "infix wildcard with sub-node tsr at depth 1 but direct match", + routes: []string{ + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbb", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/*{any}/bbb", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, + }, + { + name: "infix wildcard with sub-node tsr at depth 1 and 2 but direct match", + routes: []string{ + "/foo/*{any}/b/c/bbb/", + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbb", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/*{any}/bbb", + wantTsr: false, + wantParams: Params{ + { + Key: "any", + Value: "a/b/c", + }, + }, + }, + { + name: "infix wildcard with sub-node tsr at depth 1 and 2 but fallback first tsr", + routes: []string{ + "/foo/*{any}/b/c/bbb/", + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbx", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/*{any}/b/c/bbb/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a", + }, + }, + }, + { + name: "infix wildcard with sub-node tsr at depth 1 and 2 but fallback first tsr", + routes: []string{ + "/foo/*{any}/", + "/foo/*{any}/b/c/bbb/", + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbx", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/*{any}/b/c/bbb/", + wantTsr: true, + wantParams: Params{ + { + Key: "any", + Value: "a", + }, + }, + }, + { + name: "infix wildcard with depth 0 tsr and sub-node tsr at depth 1 fallback first tsr", + routes: []string{ + "/foo/a/b/c/bbb/", + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbx", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/a/b/c/bbb/", + wantTsr: true, + }, + { + name: "infix wildcard with depth 0 tsr and sub-node tsr at depth 1 fallback first tsr", + routes: []string{ + "/foo/{first}/b/c/bbb/", + "/foo/*{any}/c/bbb/", + "/foo/*{any}/bbx", + }, + path: "/foo/a/b/c/bbb", + wantPath: "/foo/{first}/b/c/bbb/", + wantTsr: true, + wantParams: Params{ + { + Key: "first", + Value: "a", + }, + }, + }, + { + name: "multi infix wildcard with sub-node tsr at depth 1 but direct match", + routes: []string{ + "/foo/*{any1}/b/c/*{any2}/d/", + "/foo/*{any1}/c/*{any2}/d", + }, + path: "/foo/a/b/c/x/y/z/d", + wantPath: "/foo/*{any1}/c/*{any2}/d", + wantTsr: false, + wantParams: Params{ + { + Key: "any1", + Value: "a/b", + }, + { + Key: "any2", + Value: "x/y/z", + }, + }, + }, + { + name: "multi infix wildcard with sub-node tsr at depth 1 and fallback first", + routes: []string{ + "/foo/*{any1}/b/c/*{any2}/d/", + "/foo/*{any1}/c/*{any2}/x", + }, + path: "/foo/a/b/c/x/y/z/d", + wantPath: "/foo/*{any1}/b/c/*{any2}/d/", + wantTsr: true, + wantParams: Params{ + { + Key: "any1", + Value: "a", + }, + { + Key: "any2", + Value: "x/y/z", + }, + }, + }, + { + name: "multi infix wildcard with sub-node tsr and skipped nodes at depth 1 and fallback first", + routes: []string{ + "/foo/*{any1}/b/c/*{any2}/{a}/", + "/foo/*{any1}/b/c/*{any2}/d{a}/", + "/foo/*{any1}/b/c/*{any2}/dd/", + "/foo/*{any1}/c/*{any2}/x", + }, + path: "/foo/a/b/c/x/y/z/dd", + wantPath: "/foo/*{any1}/b/c/*{any2}/dd/", + wantTsr: true, + wantParams: Params{ + { + Key: "any1", + Value: "a", + }, + { + Key: "any2", + Value: "x/y/z", + }, + }, + }, + { + name: "multi infix wildcard with sub-node tsr and skipped nodes at depth 1 and direct match", + routes: []string{ + "/foo/*{any1}/b/c/*{any2}/{a}/", + "/foo/*{any1}/b/c/*{any2}/d{a}/", + "/foo/*{any1}/b/c/*{any2}/dd/", + "/foo/*{any1}/c/*{any2}/x", + }, + path: "/foo/a/b/c/x/y/z/xd/", + wantPath: "/foo/*{any1}/b/c/*{any2}/{a}/", + wantTsr: false, + wantParams: Params{ + { + Key: "any1", + Value: "a", + }, + { + Key: "any2", + Value: "x/y/z", + }, + { + Key: "a", + Value: "xd", + }, + }, + }, + { + name: "multi infix wildcard with sub-node tsr and skipped nodes at depth 1 with direct match depth 0", + routes: []string{ + "/foo/*{any1}/b/c/*{any2}/{a}/", + "/foo/*{any1}/b/c/*{any2}/d{a}/", + "/foo/*{any1}/b/c/*{any2}/dd/", + "/foo/*{any1}/c/*{any2}/x", + "/{a}/*{any1}/c/x/y/z/dd", + }, + path: "/foo/a/b/c/x/y/z/dd", + wantPath: "/{a}/*{any1}/c/x/y/z/dd", + wantTsr: false, + wantParams: Params{ + { + Key: "a", + Value: "foo", + }, + { + Key: "any1", + Value: "a/b", + }, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + f := New() + tree := f.Tree() + for _, rte := range tc.routes { + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + } + nds := *tree.nodes.Load() + c := newTestContextTree(tree) + n, tsr := tree.lookup(nds[0].children[0].Load(), tc.path, c, false) + require.NotNil(t, n) + assert.Equal(t, tc.wantPath, n.route.path) + assert.Equal(t, tc.wantTsr, tsr) + c.tsr = tsr + assert.Equal(t, tc.wantParams, slices.Collect(c.Params())) + }) + } +} + +func TestInsertConflict(t *testing.T) { + cases := []struct { + name string + routes []struct { + wantErr error + path string + wantMatch []string + } + }{ + { + name: "exact match conflict", + routes: []struct { + wantErr error + path string + wantMatch []string + }{ + {path: "/john/*{x}", wantErr: nil, wantMatch: nil}, + {path: "/john/*{y}", wantErr: ErrRouteConflict, wantMatch: []string{"/john/*{x}"}}, + {path: "/john/", wantErr: nil, wantMatch: nil}, + {path: "/foo/baz", wantErr: nil, wantMatch: nil}, + {path: "/foo/bar", wantErr: nil, wantMatch: nil}, + {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, + {path: "/foo/*{args}", wantErr: nil, wantMatch: nil}, + {path: "/avengers/ironman/{power}", wantErr: nil, wantMatch: nil}, + {path: "/avengers/{id}/bar", wantErr: nil, wantMatch: nil}, + {path: "/avengers/{id}/foo", wantErr: nil, wantMatch: nil}, + {path: "/avengers/*{args}", wantErr: nil, wantMatch: nil}, + {path: "/fox/", wantErr: nil, wantMatch: nil}, + {path: "/fox/*{args}", wantErr: nil, wantMatch: nil}, + }, + }, + { + name: "no conflict for incomplete match to end of edge", + routes: []struct { + wantErr error + path string + wantMatch []string + }{ + {path: "/foo/bar", wantErr: nil, wantMatch: nil}, + {path: "/foo/baz", wantErr: nil, wantMatch: nil}, + {path: "/foo/*{args}", wantErr: nil, wantMatch: nil}, + {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, + }, + }, + { + name: "no conflict for key match mid-edge", + routes: []struct { + wantErr error + path string + wantMatch []string + }{ + {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, + {path: "/foo/*{args}", wantErr: nil, wantMatch: nil}, + {path: "/foo/a*{args}", wantErr: nil, wantMatch: nil}, + {path: "/foo*{args}", wantErr: nil, wantMatch: nil}, + {path: "/john{doe}", wantErr: nil, wantMatch: nil}, + {path: "/john*{doe}", wantErr: nil, wantMatch: nil}, + {path: "/john/{doe}", wantErr: nil, wantMatch: nil}, + {path: "/joh{doe}", wantErr: nil, wantMatch: nil}, + {path: "/avengers/{id}/foo", wantErr: nil, wantMatch: nil}, + {path: "/avengers/{id}/bar", wantErr: nil, wantMatch: nil}, + {path: "/avengers/*{args}", wantErr: nil, wantMatch: nil}, + }, + }, + { + name: "incomplete match to middle of edge", + routes: []struct { + wantErr error + path string + wantMatch []string + }{ + {path: "/foo/{id}", wantErr: nil, wantMatch: nil}, + {path: "/foo/{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/foo/{id}"}}, + {path: "/gchq/*{id}", wantErr: nil, wantMatch: nil}, + {path: "/gchq/*{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/gchq/*{id}"}}, + {path: "/foo{id}", wantErr: nil, wantMatch: nil}, + {path: "/foo/a{id}", wantErr: nil, wantMatch: nil}, + {path: "/avengers/{id}/bar", wantErr: nil, wantMatch: nil}, + {path: "/avengers/{id}/baz", wantErr: nil, wantMatch: nil}, + {path: "/avengers/{id}", wantErr: nil, wantMatch: nil}, + {path: "/avengers/{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/avengers/{id}", "/avengers/{id}/bar", "/avengers/{id}/baz"}}, + {path: "/ironman/*{id}/bar", wantErr: nil, wantMatch: nil}, + {path: "/ironman/*{id}/baz", wantErr: nil, wantMatch: nil}, + {path: "/ironman/*{id}", wantErr: nil, wantMatch: nil}, + {path: "/ironman/*{abc}", wantErr: ErrRouteConflict, wantMatch: []string{"/ironman/*{id}", "/ironman/*{id}/bar", "/ironman/*{id}/baz"}}, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + tree := New().Tree() + for _, rte := range tc.routes { + r, err := tree.Handle(http.MethodGet, rte.path, emptyHandler) + if err != nil { + assert.Nil(t, r) + } + assert.ErrorIsf(t, err, rte.wantErr, "route: %s", rte.path) + if cErr, ok := err.(*RouteConflictError); ok { + assert.Equal(t, rte.wantMatch, cErr.Matched) + } + } + }) + } +} + +func TestUpdateConflict(t *testing.T) { + cases := []struct { + name string + routes []string + update string + wantErr error + wantMatch []string + }{ + { + name: "wildcard parameter route not registered", + routes: []string{"/foo/{bar}"}, + update: "/foo/{baz}", + wantErr: ErrRouteNotFound, + }, + { + name: "wildcard catch all route not registered", + routes: []string{"/foo/{bar}"}, + update: "/foo/*{baz}", + wantErr: ErrRouteNotFound, + }, + { + name: "route match but not a leaf", + routes: []string{"/foo/bar/baz"}, + update: "/foo/bar", + wantErr: ErrRouteNotFound, + }, + { + name: "wildcard have different name", + routes: []string{"/foo/bar", "/foo/*{args}"}, + update: "/foo/*{all}", + wantErr: ErrRouteNotFound, + wantMatch: []string{"/foo/*{args}"}, + }, + { + name: "replacing non wildcard by wildcard", + routes: []string{"/foo/bar", "/foo/"}, + update: "/foo/*{all}", + wantErr: ErrRouteNotFound, + wantMatch: []string{"/foo/"}, + }, + { + name: "replacing wildcard by non wildcard", + routes: []string{"/foo/bar", "/foo/*{args}"}, + update: "/foo/", + wantErr: ErrRouteNotFound, + wantMatch: []string{"/foo/*{args}"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + tree := New().Tree() + for _, rte := range tc.routes { + require.NoError(t, onlyError(tree.Handle(http.MethodGet, rte, emptyHandler))) + } + r, err := tree.Update(http.MethodGet, tc.update, emptyHandler) + if err != nil { + assert.Nil(t, r) } + assert.ErrorIs(t, err, tc.wantErr) }) } } @@ -1488,6 +2678,16 @@ func TestUpdateRoute(t *testing.T) { routes: []string{"/foo/*{bar}", "/foo", "/foo/bar"}, update: "/foo/*{bar}", }, + { + name: "replacing infix catch all node", + routes: []string{"/foo/*{bar}/baz", "/foo", "/foo/bar"}, + update: "/foo/*{bar}/baz", + }, + { + name: "replacing infix inflight catch all node", + routes: []string{"/foo/abc*{bar}/baz", "/foo", "/foo/abc{bar}"}, + update: "/foo/abc*{bar}/baz", + }, } for _, tc := range cases { @@ -1503,145 +2703,184 @@ func TestUpdateRoute(t *testing.T) { func TestParseRoute(t *testing.T) { cases := []struct { - wantErr error - name string - path string - wantCatchAllKey string - wantPath string - wantN int + wantErr error + name string + path string + wantN uint32 }{ { - name: "valid static route", - path: "/foo/bar", - wantPath: "/foo/bar", + name: "valid static route", + path: "/foo/bar", + }, + { + name: "valid catch all route", + path: "/foo/bar/*{arg}", + wantN: 1, }, { - name: "valid catch all route", - path: "/foo/bar/*{arg}", - wantN: 1, - wantCatchAllKey: "arg", - wantPath: "/foo/bar/", + name: "valid param route", + path: "/foo/bar/{baz}", + wantN: 1, }, { - name: "valid param route", - path: "/foo/bar/{baz}", - wantN: 1, - wantPath: "/foo/bar/{baz}", + name: "valid multi params route", + path: "/foo/{bar}/{baz}", + wantN: 2, }, { - name: "valid multi params route", - path: "/foo/{bar}/{baz}", - wantN: 2, - wantPath: "/foo/{bar}/{baz}", + name: "valid same params route", + path: "/foo/{bar}/{bar}", + wantN: 2, }, { - name: "valid same params route", - path: "/foo/{bar}/{bar}", - wantN: 2, - wantPath: "/foo/{bar}/{bar}", + name: "valid multi params and catch all route", + path: "/foo/{bar}/{baz}/*{arg}", + wantN: 3, }, { - name: "valid multi params and catch all route", - path: "/foo/{bar}/{baz}/*{arg}", - wantN: 3, - wantCatchAllKey: "arg", - wantPath: "/foo/{bar}/{baz}/", + name: "valid inflight param", + path: "/foo/xyz:{bar}", + wantN: 1, }, { - name: "valid inflight param", - path: "/foo/xyz:{bar}", - wantN: 1, - wantPath: "/foo/xyz:{bar}", + name: "valid inflight catchall", + path: "/foo/xyz:*{bar}", + wantN: 1, }, { - name: "valid inflight catchall", - path: "/foo/xyz:*{bar}", - wantN: 1, - wantPath: "/foo/xyz:", - wantCatchAllKey: "bar", + name: "valid multi inflight param and catch all", + path: "/foo/xyz:{bar}/abc:{bar}/*{arg}", + wantN: 3, }, { - name: "valid multi inflight param and catch all", - path: "/foo/xyz:{bar}/abc:{bar}/*{arg}", - wantN: 3, - wantCatchAllKey: "arg", - wantPath: "/foo/xyz:{bar}/abc:{bar}/", + name: "catch all with arg in the middle of the route", + path: "/foo/bar/*{bar}/baz", + wantN: 1, + }, + { + name: "multiple catch all suffix and inflight with arg in the middle of the route", + path: "/foo/bar/*{bar}/x*{args}/y/*{z}/{b}", + wantN: 4, + }, + { + name: "inflight catch all with arg in the middle of the route", + path: "/foo/bar/damn*{bar}/baz", + wantN: 1, + }, + { + name: "catch all with arg in the middle of the route and param after", + path: "/foo/bar/*{bar}/{baz}", + wantN: 2, }, { name: "missing prefix slash", path: "foo/bar", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "empty parameter", path: "/foo/bar{}", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "missing arguments name after catch all", path: "/foo/bar/*", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "missing arguments name after param", path: "/foo/bar/{", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "catch all in the middle of the route", path: "/foo/bar/*/baz", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, + }, + { + name: "empty infix catch all", + path: "/foo/bar/*{}/baz", + wantErr: ErrInvalidRoute, + wantN: 0, }, { - name: "catch all with arg in the middle of the route", - path: "/foo/bar/*{bar}/baz", + name: "empty ending catch all", + path: "/foo/bar/baz/*{}", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "unexpected character in param", path: "/foo/{{bar}", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "unexpected character in param", path: "/foo/{*bar}", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "in flight catch-all after param in one route segment", path: "/foo/{bar}*{baz}", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "multiple param in one route segment", path: "/foo/{bar}{baz}", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, }, { name: "in flight param after catch all", path: "/foo/*{args}{param}", wantErr: ErrInvalidRoute, - wantN: -1, + wantN: 0, + }, + { + name: "consecutive catch all with no slash", + path: "/foo/*{args}*{param}", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "consecutive catch all", + path: "/foo/*{args}/*{param}", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "consecutive catch all with inflight", + path: "/foo/ab*{args}/*{param}", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "unexpected char after inflight catch all", + path: "/foo/ab*{args}a", + wantErr: ErrInvalidRoute, + wantN: 0, + }, + { + name: "unexpected char after catch all", + path: "/foo/*{args}a", + wantErr: ErrInvalidRoute, + wantN: 0, }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - path, key, n, err := parseRoute(tc.path) + n, err := parseRoute(tc.path) require.ErrorIs(t, err, tc.wantErr) assert.Equal(t, tc.wantN, n) - assert.Equal(t, tc.wantCatchAllKey, key) - assert.Equal(t, tc.wantPath, path) }) } } @@ -1718,11 +2957,11 @@ func TestTree_LookupTsr(t *testing.T) { t.Run(tc.name, func(t *testing.T) { tree := New().Tree() for _, path := range tc.paths { - require.NoError(t, tree.insert(http.MethodGet, path, "", 0, tree.newRoute(path, emptyHandler))) + require.NoError(t, tree.insert(http.MethodGet, tree.newRoute(path, emptyHandler), 0)) } nds := *tree.nodes.Load() c := newTestContextTree(tree) - n, got := tree.lookup(nds[0], tc.key, c, true) + n, got := tree.lookup(nds[0].children[0].Load(), tc.key, c, true) assert.Equal(t, tc.want, got) if tc.want { require.NotNil(t, n) @@ -3067,11 +4306,11 @@ func TestFuzzInsertLookupParam(t *testing.T) { continue } path := fmt.Sprintf(routeFormat, s1, e1, s2, e2, e3) - if err := tree.insert(http.MethodGet, path, "", 3, tree.newRoute(path, emptyHandler)); err == nil { + if err := tree.insert(http.MethodGet, tree.newRoute(path, emptyHandler), 3); err == nil { nds := *tree.nodes.Load() c := newTestContextTree(tree) - n, tsr := tree.lookup(nds[0], fmt.Sprintf(reqFormat, s1, "xxxx", s2, "xxxx", "xxxx"), c, false) + n, tsr := tree.lookup(nds[0].children[0].Load(), fmt.Sprintf(reqFormat, s1, "xxxx", s2, "xxxx", "xxxx"), c, false) require.NotNil(t, n) require.NotNil(t, n.route) assert.False(t, tsr) @@ -3091,14 +4330,12 @@ func TestFuzzInsertNoPanics(t *testing.T) { f.Fuzz(&routes) for rte := range routes { - var catchAllKey string - f.Fuzz(&catchAllKey) if rte == "" { continue } require.NotPanicsf(t, func() { - _ = tree.insert(http.MethodGet, rte, catchAllKey, 0, tree.newRoute(appendCatchAll(rte, catchAllKey), emptyHandler)) - }, fmt.Sprintf("rte: %s, catch all: %s", rte, catchAllKey)) + _ = tree.insert(http.MethodGet, tree.newRoute(rte, emptyHandler), 0) + }, fmt.Sprintf("rte: %s", rte)) } } @@ -3119,7 +4356,7 @@ func TestFuzzInsertLookupUpdateAndDelete(t *testing.T) { for rte := range routes { path := "/" + rte - err := tree.insert(http.MethodGet, path, "", 0, tree.newRoute(path, emptyHandler)) + err := tree.insert(http.MethodGet, tree.newRoute(path, emptyHandler), 0) require.NoError(t, err) } @@ -3130,18 +4367,18 @@ func TestFuzzInsertLookupUpdateAndDelete(t *testing.T) { for rte := range routes { nds := *tree.nodes.Load() c := newTestContextTree(tree) - n, tsr := tree.lookup(nds[0], "/"+rte, c, true) + n, tsr := tree.lookup(nds[0].children[0].Load(), "/"+rte, c, true) require.NotNilf(t, n, "route /%s", rte) require.NotNilf(t, n.route, "route /%s", rte) require.Falsef(t, tsr, "tsr: %t", tsr) require.Truef(t, n.isLeaf(), "route /%s", rte) require.Equal(t, "/"+rte, n.route.path) path := "/" + rte - require.NoError(t, tree.update(http.MethodGet, path, "", tree.newRoute(path, emptyHandler))) + require.NoError(t, tree.update(http.MethodGet, tree.newRoute(path, emptyHandler))) } for rte := range routes { - deleted := tree.remove(http.MethodGet, "/"+rte, "") + deleted := tree.remove(http.MethodGet, "/"+rte) require.True(t, deleted) } @@ -3221,7 +4458,7 @@ func TestTree_RaceDetector(t *testing.T) { } wg.Done() }() - tree.insert(rte.method, rte.path, "", 0, &Route{path: rte.path}) + _ = tree.insert(rte.method, &Route{path: rte.path}, 0) }() go func() { @@ -3233,7 +4470,7 @@ func TestTree_RaceDetector(t *testing.T) { } wg.Done() }() - tree.update(rte.method, rte.path, "", &Route{path: rte.path}) + _ = tree.update(rte.method, &Route{path: rte.path}) }() go func() { @@ -3245,7 +4482,7 @@ func TestTree_RaceDetector(t *testing.T) { } wg.Done() }() - tree.remove(rte.method, rte.path, "") + tree.remove(rte.method, rte.path) }() } @@ -3343,7 +4580,7 @@ func TestNode_String(t *testing.T) { nds := *tree.nodes.Load() want := `path: GET - path: /foo/{bar}/ [catchAll] [leaf=/foo/{bar}/*{baz}] [bar (10)]` + path: /foo/{bar}/*{baz} [leaf=/foo/{bar}/*{baz}] [bar (10), baz (-1)]` assert.Equal(t, want, strings.TrimSuffix(nds[0].String(), "\n")) } diff --git a/iter.go b/iter.go index 50b50be..f2e5c3b 100644 --- a/iter.go +++ b/iter.go @@ -95,7 +95,7 @@ func (it Iter) Routes(methods iter.Seq[string], path string) iter.Seq2[string, * continue } - n, tsr := it.t.lookup(nds[index], path, c, true) + n, tsr := it.t.lookup(nds[index].children[0].Load(), path, c, true) if n != nil && !tsr && n.route.path == path { if !yield(method, n.route) { return @@ -128,7 +128,7 @@ func (it Iter) Reverse(methods iter.Seq[string], path string) iter.Seq2[string, continue } - n, tsr := it.t.lookup(nds[index], path, c, true) + n, tsr := it.t.lookup(nds[index].children[0].Load(), path, c, true) if n != nil && (!tsr || n.route.redirectTrailingSlash || n.route.ignoreTrailingSlash) { if !yield(method, n.route) { return diff --git a/node.go b/node.go index 4d8e1bf..3c7e0cb 100644 --- a/node.go +++ b/node.go @@ -18,12 +18,9 @@ type node struct { route *Route // key represent a segment of a route which share a common prefix with it parent. + // Once assigned, key is immutable. key string - // Catch all key registered to retrieve this node parameter. - // Once assigned, catchAllKey is immutable. - catchAllKey string - // First char of each outgoing edges from this node sorted in ascending order. // Once assigned, this is a read only slice. It allows to lazily traverse the // tree without the extra cost of atomic load operation. @@ -37,37 +34,43 @@ type node struct { params []param // The index of a paramChild if any, -1 if none (per rules, only one paramChildren is allowed). - paramChildIndex int + // Once assigned, paramChildIndex is immutable. + paramChildIndex int // TODO uint32 + // The index of a wildcardChild if any, -1 if none (per rules, only one wildcardChild is allowed). + // Once assigned, wildcardChildIndex is immutable. + wildcardChildIndex int } -func newNode(key string, route *Route, children []*node, catchAllKey string) *node { +func newNode(key string, route *Route, children []*node) *node { slices.SortFunc(children, func(a, b *node) int { return cmp.Compare(a.key, b.key) }) nds := make([]atomic.Pointer[node], len(children)) childKeys := make([]byte, len(children)) - childIndex := -1 + paramChildIndex := -1 + wildcardChildIndex := -1 for i := range children { assertNotNil(children[i]) childKeys[i] = children[i].key[0] nds[i].Store(children[i]) if strings.HasPrefix(children[i].key, "{") { - childIndex = i + paramChildIndex = i + } else if strings.HasPrefix(children[i].key, "*") { + wildcardChildIndex = i } } - - return newNodeFromRef(key, route, nds, childKeys, catchAllKey, childIndex) + return newNodeFromRef(key, route, nds, childKeys, paramChildIndex, wildcardChildIndex) } -func newNodeFromRef(key string, route *Route, children []atomic.Pointer[node], childKeys []byte, catchAllKey string, childIndex int) *node { +func newNodeFromRef(key string, route *Route, children []atomic.Pointer[node], childKeys []byte, paramChildIndex, wildcardChildIndex int) *node { return &node{ - key: key, - childKeys: childKeys, - children: children, - route: route, - catchAllKey: catchAllKey, - paramChildIndex: childIndex, - params: parseWildcard(key), + key: key, + childKeys: childKeys, + children: children, + route: route, + paramChildIndex: paramChildIndex, + wildcardChildIndex: wildcardChildIndex, + params: parseWildcard(key), } } @@ -75,10 +78,6 @@ func (n *node) isLeaf() bool { return n.route != nil } -func (n *node) isCatchAll() bool { - return n.catchAllKey != "" -} - func (n *node) hasWildcard() bool { return len(n.params) > 0 } @@ -188,25 +187,14 @@ func (n *node) string(space int) string { sb.WriteString(" [paramIdx=") sb.WriteString(strconv.Itoa(n.paramChildIndex)) sb.WriteByte(']') - if n.hasWildcard() { - sb.WriteString(" [") - for i, param := range n.params { - if i > 0 { - sb.WriteByte(',') - } - sb.WriteString(param.key) - sb.WriteString(" (") - sb.WriteString(strconv.Itoa(param.end)) - sb.WriteString(")") - } - sb.WriteString("]") - } - } - if n.isCatchAll() { - sb.WriteString(" [catchAll]") + if n.wildcardChildIndex >= 0 { + sb.WriteString(" [wildcardIdx=") + sb.WriteString(strconv.Itoa(n.wildcardChildIndex)) + sb.WriteByte(']') } + if n.isLeaf() { sb.WriteString(" [leaf=") sb.WriteString(n.route.path) @@ -216,7 +204,7 @@ func (n *node) string(space int) string { sb.WriteString(" [") for i, param := range n.params { if i > 0 { - sb.WriteByte(',') + sb.WriteString(", ") } sb.WriteString(param.key) sb.WriteString(" (") @@ -244,27 +232,19 @@ func (n *skippedNodes) pop() skippedNode { } type skippedNode struct { - n *node - pathIndex int - paramCnt uint32 - seen bool -} - -func appendCatchAll(path, catchAllKey string) string { - if catchAllKey != "" { - suffix := "*{" + catchAllKey + "}" - if !strings.HasSuffix(path, suffix) { - return path + suffix - } - } - return path + n *node + pathIndex int + paramCnt uint32 + childIndex int } // param represents a parsed parameter and its end position in the path. type param struct { key string - end int // -1 if end with {a}, else pos of the next char - // catchAll bool + // -1 if end with {a}, else pos of the next char. Note that since infix wildcard are always followed + // by a static segment, pos should be always > 0 + end int + catchAll bool } func parseWildcard(segment string) []param { @@ -289,28 +269,28 @@ func parseWildcard(segment string) []param { state = stateDefault } i++ - //case stateCatchAll: - //if segment[i] == '}' { - // end := -1 - // if len(segment[i+1:]) > 0 { - // end = i + 1 - // } - // params = append(params, param{ - // key: segment[start:i], - // end: end, - // catchAll: true, - // }) - // start = 0 - // state = stateDefault - //} - //i++ + case stateCatchAll: + if segment[i] == '}' { + end := -1 + if len(segment[i+1:]) > 0 { + end = i + 1 + } + params = append(params, param{ + key: segment[start:i], + end: end, + catchAll: true, + }) + start = 0 + state = stateDefault + } + i++ default: - // if segment[i] == '*' { - // state = stateCatchAll - // i += 2 - // start = i - // continue - //} + if segment[i] == '*' { + state = stateCatchAll + i += 2 + start = i + continue + } if segment[i] == '{' { state = stateParam diff --git a/options.go b/options.go index ada69f9..b66ad81 100644 --- a/options.go +++ b/options.go @@ -233,13 +233,6 @@ func WithAnnotations(annotations ...Annotation) PathOption { }) } -// WithAnnotation attaches a single key-value annotation to a route. See also [WithAnnotations] and [Annotation] for more details. -func WithAnnotation(key string, value any) PathOption { - return pathOptionFunc(func(route *Route) { - route.annots = append(route.annots, Annotation{key, value}) - }) -} - // DefaultOptions configure the router to use the Recovery middleware for the RouteHandler scope, the Logger middleware // for AllHandlers scope and enable automatic OPTIONS response. Note that DefaultOptions push the Recovery and Logger middleware // respectively to the first and second position of the middleware chains. diff --git a/route.go b/route.go index 2847340..a1c4abe 100644 --- a/route.go +++ b/route.go @@ -5,9 +5,6 @@ import ( "strings" ) -// Annotations is a collection of Annotation key-value pairs that can be attached to routes. -type Annotations []Annotation - // Annotation represents a single key-value pair that provides metadata for a route. // Annotations are typically used to store information that can be leveraged by middleware, handlers, or external // libraries to modify or customize route behavior. @@ -25,7 +22,7 @@ type Route struct { hall HandlerFunc path string mws []middleware - annots Annotations + annots []Annotation redirectTrailingSlash bool ignoreTrailingSlash bool } diff --git a/tree.go b/tree.go index 666cce8..da3dfdc 100644 --- a/tree.go +++ b/tree.go @@ -6,7 +6,6 @@ package fox import ( "fmt" - "math" "net/http" "slices" "strings" @@ -57,19 +56,14 @@ func (t *Tree) Handle(method, path string, handler HandlerFunc, opts ...PathOpti return nil, fmt.Errorf("%w: missing or invalid http method", ErrInvalidRoute) } - p, catchAllKey, n, err := parseRoute(path) + n, err := parseRoute(path) if err != nil { return nil, err } - if n < 0 || n > math.MaxUint32 { - return nil, fmt.Errorf("params count overflows (%d)", n) - } - rte := t.newRoute(path, handler, opts...) - // nolint:gosec - if err = t.insert(method, p, catchAllKey, uint32(n), rte); err != nil { + if err = t.insert(method, rte, n); err != nil { return nil, err } return rte, nil @@ -91,13 +85,13 @@ func (t *Tree) Update(method, path string, handler HandlerFunc, opts ...PathOpti return nil, fmt.Errorf("%w: missing http method", ErrInvalidRoute) } - p, catchAllKey, _, err := parseRoute(path) + _, err := parseRoute(path) if err != nil { return nil, err } rte := t.newRoute(path, handler, opts...) - if err = t.update(method, p, catchAllKey, rte); err != nil { + if err = t.update(method, rte); err != nil { return nil, err } @@ -114,12 +108,12 @@ func (t *Tree) Delete(method, path string) error { return fmt.Errorf("%w: missing http method", ErrInvalidRoute) } - p, catchAllKey, _, err := parseRoute(path) + _, err := parseRoute(path) if err != nil { return err } - if !t.remove(method, p, catchAllKey) { + if !t.remove(method, path) { return fmt.Errorf("%w: route %s %s is not registered", ErrRouteNotFound, method, path) } @@ -140,13 +134,13 @@ func (t *Tree) Has(method, path string) bool { func (t *Tree) Route(method, path string) *Route { nds := *t.nodes.Load() index := findRootNode(method, nds) - if index < 0 { + if index < 0 || len(nds[index].children) == 0 { return nil } c := t.ctx.Get().(*cTx) c.resetNil() - n, tsr := t.lookup(nds[index], path, c, true) + n, tsr := t.lookup(nds[index].children[0].Load(), path, c, true) c.Close() if n != nil && !tsr && n.route.path == path { return n.route @@ -162,13 +156,13 @@ func (t *Tree) Route(method, path string) *Route { func (t *Tree) Reverse(method, path string) (route *Route, tsr bool) { nds := *t.nodes.Load() index := findRootNode(method, nds) - if index < 0 { + if index < 0 || len(nds[index].children) == 0 { return nil, false } c := t.ctx.Get().(*cTx) c.resetNil() - n, tsr := t.lookup(nds[index], path, c, true) + n, tsr := t.lookup(nds[index].children[0].Load(), path, c, true) c.Close() if n != nil { return n.route, tsr @@ -185,8 +179,7 @@ func (t *Tree) Reverse(method, path string) (route *Route, tsr bool) { func (t *Tree) Lookup(w ResponseWriter, r *http.Request) (route *Route, cc ContextCloser, tsr bool) { nds := *t.nodes.Load() index := findRootNode(r.Method, nds) - - if index < 0 { + if index < 0 || len(nds[index].children) == 0 { return } @@ -199,7 +192,7 @@ func (t *Tree) Lookup(w ResponseWriter, r *http.Request) (route *Route, cc Conte target = r.URL.RawPath } - n, tsr := t.lookup(nds[index], target, c, false) + n, tsr := t.lookup(nds[index].children[0].Load(), target, c, false) if n != nil { c.route = n.route c.tsr = tsr @@ -218,7 +211,7 @@ func (t *Tree) Iter() Iter { // Insert is not safe for concurrent use. The path must start by '/' and it's not validated. Use // parseRoute before. -func (t *Tree) insert(method, path, catchAllKey string, paramsN uint32, route *Route) error { +func (t *Tree) insert(method string, route *Route, paramsN uint32) error { // Note that we need a consistent view of the tree during the patching so search must imperatively be locked. if !t.race.CompareAndSwap(0, 1) { panic(ErrConcurrentAccess) @@ -235,7 +228,7 @@ func (t *Tree) insert(method, path, catchAllKey string, paramsN uint32, route *R rootNode = nds[index] } - isCatchAll := catchAllKey != "" + path := route.path result := t.search(rootNode, path) switch result.classify() { @@ -246,15 +239,19 @@ func (t *Tree) insert(method, path, catchAllKey string, paramsN uint32, route *R // └── am // Create a new node from "st" reference and update the "te" (parent) reference to "st" node. if result.matched.isLeaf() { - if result.matched.isCatchAll() && isCatchAll { - return newConflictErr(method, path, catchAllKey, getRouteConflict(result.matched)) - } return fmt.Errorf("%w: new route %s %s conflict with %s", ErrRouteExist, method, route.path, result.matched.route.path) } // We are updating an existing node. We only need to create a new node from // the matched one with the updated/added value (handler and wildcard). - n := newNodeFromRef(result.matched.key, route, result.matched.children, result.matched.childKeys, catchAllKey, result.matched.paramChildIndex) + n := newNodeFromRef( + result.matched.key, + route, + result.matched.children, + result.matched.childKeys, + result.matched.paramChildIndex, + result.matched.wildcardChildIndex, + ) t.updateMaxParams(paramsN) result.p.updateEdge(n) @@ -284,15 +281,14 @@ func (t *Tree) insert(method, path, catchAllKey string, paramsN uint32, route *R result.matched.route, result.matched.children, result.matched.childKeys, - result.matched.catchAllKey, result.matched.paramChildIndex, + result.matched.wildcardChildIndex, ) parent := newNode( cPrefix, route, []*node{child}, - catchAllKey, ) t.updateMaxParams(paramsN) @@ -316,14 +312,13 @@ func (t *Tree) insert(method, path, catchAllKey string, paramsN uint32, route *R keySuffix := path[result.charsMatched:] // No children, so no paramChild - child := newNode(keySuffix, route, nil, catchAllKey) + child := newNode(keySuffix, route, nil) edges := result.matched.getEdgesShallowCopy() edges = append(edges, child) n := newNode( result.matched.key, result.matched.route, edges, - result.matched.catchAllKey, ) t.updateMaxDepth(result.depth + 1) @@ -359,32 +354,32 @@ func (t *Tree) insert(method, path, catchAllKey string, paramsN uint32, route *R keyCharsFromStartOfNodeFound := path[result.charsMatched-result.charsMatchedInNodeFound:] cPrefix := commonPrefix(keyCharsFromStartOfNodeFound, result.matched.key) - // Rule: a node with {param} has no child or has a separator before the end of the key + // Rule: a node with {param} or *{wildcard} has no child or has a separator before the end of the key for i := len(cPrefix) - 1; i >= 0; i-- { if cPrefix[i] == '/' { break } - if cPrefix[i] == '{' { - return newConflictErr(method, path, catchAllKey, getRouteConflict(result.matched)) + if cPrefix[i] == '{' || cPrefix[i] == '*' { + return newConflictErr(method, path, getRouteConflict(result.matched)) } } suffixFromExistingEdge := strings.TrimPrefix(result.matched.key, cPrefix) keySuffix := path[result.charsMatched:] - // No children, so no paramChild - n1 := newNodeFromRef(keySuffix, route, nil, nil, catchAllKey, -1) // inserted node + // No children, so no paramChild or wildcardChild + n1 := newNodeFromRef(keySuffix, route, nil, nil, -1, -1) // inserted node n2 := newNodeFromRef( suffixFromExistingEdge, result.matched.route, result.matched.children, result.matched.childKeys, - result.matched.catchAllKey, result.matched.paramChildIndex, + result.matched.wildcardChildIndex, ) // previous matched node // n3 children never start with a param - n3 := newNode(cPrefix, nil, []*node{n1, n2}, "") // intermediary node + n3 := newNode(cPrefix, nil, []*node{n1, n2}) // intermediary node t.updateMaxDepth(result.depth + 1) t.updateMaxParams(paramsN) @@ -397,13 +392,15 @@ func (t *Tree) insert(method, path, catchAllKey string, paramsN uint32, route *R } // update is not safe for concurrent use. -func (t *Tree) update(method string, path, catchAllKey string, route *Route) error { +func (t *Tree) update(method string, route *Route) error { // Note that we need a consistent view of the tree during the patching so search must imperatively be locked. if !t.race.CompareAndSwap(0, 1) { panic(ErrConcurrentAccess) } defer t.race.Store(0) + path := route.path + nds := *t.nodes.Load() index := findRootNode(method, nds) if index < 0 { @@ -415,12 +412,6 @@ func (t *Tree) update(method string, path, catchAllKey string, route *Route) err return fmt.Errorf("%w: route %s %s is not registered", ErrRouteNotFound, method, path) } - if catchAllKey != result.matched.catchAllKey { - err := newConflictErr(method, path, catchAllKey, []string{result.matched.route.path}) - err.isUpdate = true - return err - } - // We are updating an existing node (could be a leaf or not). We only need to create a new node from // the matched one with the updated/added value (handler and wildcard). n := newNodeFromRef( @@ -428,15 +419,15 @@ func (t *Tree) update(method string, path, catchAllKey string, route *Route) err route, result.matched.children, result.matched.childKeys, - catchAllKey, result.matched.paramChildIndex, + result.matched.wildcardChildIndex, ) result.p.updateEdge(n) return nil } // remove is not safe for concurrent use. -func (t *Tree) remove(method, path, catchAllKey string) bool { +func (t *Tree) remove(method, path string) bool { // Note that we need a consistent view of the tree during the patching so search must imperatively be locked. if !t.race.CompareAndSwap(0, 1) { panic(ErrConcurrentAccess) @@ -450,7 +441,7 @@ func (t *Tree) remove(method, path, catchAllKey string) bool { } result := t.search(nds[index], path) - if result.classify() != exactMatch || catchAllKey != result.matched.catchAllKey { + if result.classify() != exactMatch { return false } @@ -466,8 +457,8 @@ func (t *Tree) remove(method, path, catchAllKey string) bool { nil, result.matched.children, result.matched.childKeys, - "", result.matched.paramChildIndex, + result.matched.wildcardChildIndex, ) result.p.updateEdge(n) return true @@ -481,8 +472,8 @@ func (t *Tree) remove(method, path, catchAllKey string) bool { child.route, child.children, child.childKeys, - child.catchAllKey, child.paramChildIndex, + child.wildcardChildIndex, ) result.p.updateEdge(n) return true @@ -509,15 +500,14 @@ func (t *Tree) remove(method, path, catchAllKey string) bool { child.route, child.children, child.childKeys, - child.catchAllKey, child.paramChildIndex, + child.wildcardChildIndex, ) } else { parent = newNode( result.p.key, result.p.route, parentEdges, - result.p.catchAllKey, ) } @@ -538,13 +528,10 @@ func (t *Tree) remove(method, path, catchAllKey string) bool { const ( slashDelim = '/' bracketDelim = '{' + starDelim = '*' ) -func (t *Tree) lookup(rootNode *node, path string, c *cTx, lazy bool) (n *node, tsr bool) { - if len(rootNode.children) == 0 { - return nil, false - } - +func (t *Tree) lookup(target *node, path string, c *cTx, lazy bool) (n *node, tsr bool) { var ( charsMatched int charsMatchedInNodeFound int @@ -553,7 +540,7 @@ func (t *Tree) lookup(rootNode *node, path string, c *cTx, lazy bool) (n *node, parent *node ) - current := rootNode.children[0].Load() + current := target *c.skipNds = (*c.skipNds)[:0] Walk: @@ -564,8 +551,8 @@ Walk: break } - if current.key[i] != path[charsMatched] || path[charsMatched] == bracketDelim { - if current.key[i] == '{' { + if current.key[i] != path[charsMatched] || path[charsMatched] == bracketDelim || path[charsMatched] == starDelim { + if current.key[i] == bracketDelim { startPath := charsMatched idx := strings.IndexByte(path[charsMatched:], slashDelim) if idx > 0 { @@ -598,6 +585,112 @@ Walk: continue } + if current.key[i] == starDelim { + // | current.params[paramKeyCnt].end (10) + // key: foo/*{bar}/ => 10 - 5 = 5 => i+=idx set i to '/' + // | charsMatchedInNodeFound (5) + idx := current.params[paramKeyCnt].end - charsMatchedInNodeFound + var interNode *node + if idx >= 0 { + // Unfortunately, we cannot use object pooling here because we need to keep a reference to this + // interNode object until the lookup function returns, especially when TSR (Trailing Slash Redirect) + // is enabled. The interNode may be referenced by subNode and 'n'. + interNode = &node{ + route: current.route, + key: current.key[current.params[paramKeyCnt].end:], + childKeys: current.childKeys, + children: current.children, + // len(current.params)-1 is safe since we have at least the current infix wildcard in params + params: make([]param, 0, len(current.params)-1), + paramChildIndex: current.paramChildIndex, + wildcardChildIndex: current.wildcardChildIndex, + } + for _, ps := range current.params[paramKeyCnt+1:] { // paramKeyCnt+1 is safe since we have at least the current infix wildcard in params + interNode.params = append(interNode.params, param{ + key: ps.key, + // end is relative to the original key, so we need to adjust the position relative to + // the new intermediary node. + end: ps.end - current.params[paramKeyCnt].end, + catchAll: ps.catchAll, + }) + } + + charsMatchedInNodeFound += idx + + } else if len(current.children) > 0 { + interNode = current.get(0) + charsMatchedInNodeFound += len(current.key[charsMatchedInNodeFound:]) + } else { + // We are in an ending catch all node with no child, so it's a direct match + if !lazy { + *c.params = append(*c.params, Param{Key: current.params[paramKeyCnt].key, Value: path[charsMatched:]}) + } + return current, false + } + + subCtx := t.ctx.Get().(*cTx) + startPath := charsMatched + for { + idx := strings.IndexByte(path[charsMatched:], slashDelim) + // idx >= 0, we have a next segment with at least one char + if idx > 0 { + *subCtx.params = (*subCtx.params)[:0] + charsMatched += idx + subNode, subTsr := t.lookup(interNode, path[charsMatched:], subCtx, false) + if subNode == nil { + // Try with next segment + charsMatched++ + continue + } + + // We have a tsr opportunity + if subTsr { + // Only if no previous tsr + if !tsr { + tsr = true + n = subNode + if !lazy { + *c.tsrParams = (*c.tsrParams)[:0] + *c.tsrParams = append(*c.tsrParams, *c.params...) + *c.tsrParams = append(*c.tsrParams, Param{Key: current.params[paramKeyCnt].key, Value: path[startPath:charsMatched]}) + *c.tsrParams = append(*c.tsrParams, *subCtx.tsrParams...) + } + } + + // Try with next segment + charsMatched++ + continue + } + + if !lazy { + *c.params = append(*c.params, Param{Key: current.params[paramKeyCnt].key, Value: path[startPath:charsMatched]}) + *c.params = append(*c.params, *subCtx.params...) + } + + t.ctx.Put(subCtx) + return subNode, subTsr + } + + t.ctx.Put(subCtx) + + // We can record params here because it may be either an ending catch-all node (leaf=/foo/*{args}) with + // children, or we may have a tsr opportunity (leaf=/foo/*{args}/ with /foo/x/y/z path). Note that if + // there is no tsr opportunity, and skipped nodes > 0, we will truncate the params anyway. + if !lazy { + *c.params = append(*c.params, Param{Key: current.params[paramKeyCnt].key, Value: path[startPath:]}) + } + + // We are also in an ending catch all, and this is the most specific path + if current.params[paramKeyCnt].end == -1 { + return current, false + } + + charsMatched += len(path[charsMatched:]) + + break Walk + } + } + break Walk } @@ -615,28 +708,43 @@ Walk: } } - // Only one node which is a child param, load it directly and go deeper + // No next static segment found, but maybe some params or wildcard child if idx < 0 { - if current.paramChildIndex < 0 { - break - } + // We have at least a param child which is has higher priority that catch-all + if current.paramChildIndex >= 0 { + // We have also a wildcard child, save it for later evaluation + if current.wildcardChildIndex >= 0 { + *c.skipNds = append(*c.skipNds, skippedNode{current, charsMatched, paramCnt, current.wildcardChildIndex}) + } - // The node is also a catch-all, save it as the last fallback. - if current.catchAllKey != "" { - *c.skipNds = append(*c.skipNds, skippedNode{current, charsMatched, paramCnt, true}) + // Go deeper + idx = current.paramChildIndex + parent = current + current = current.children[idx].Load() + paramKeyCnt = 0 + continue + } + if current.wildcardChildIndex >= 0 { + // We have a wildcard child, go deeper + idx = current.wildcardChildIndex + parent = current + current = current.children[idx].Load() + paramKeyCnt = 0 + continue } - idx = current.paramChildIndex - parent = current - current = current.children[idx].Load() - paramKeyCnt = 0 - continue + // We have nothing more to evaluate + break } - // Save the node if we need to evaluate the child param or catch-all later - if current.paramChildIndex >= 0 || current.catchAllKey != "" { - *c.skipNds = append(*c.skipNds, skippedNode{current, charsMatched, paramCnt, false}) + // Here we have a next static segment and possibly wildcard children, so we save them for later evaluation if needed. + if current.wildcardChildIndex >= 0 { + *c.skipNds = append(*c.skipNds, skippedNode{current, charsMatched, paramCnt, current.wildcardChildIndex}) + } + if current.paramChildIndex >= 0 { + *c.skipNds = append(*c.skipNds, skippedNode{current, charsMatched, paramCnt, current.paramChildIndex}) } + parent = current current = current.children[idx].Load() paramKeyCnt = 0 @@ -685,12 +793,6 @@ Walk: // From here we are always in a leaf if charsMatched == len(path) { if charsMatchedInNodeFound == len(current.key) { - // Exact match, note that if we match a catch-all node - if !lazy && current.catchAllKey != "" { - *c.params = append(*c.params, Param{Key: current.catchAllKey, Value: path[charsMatched:]}) - // Exact match, tsr is always false - return current, false - } // Exact match, tsr is always false return current, false } @@ -746,16 +848,6 @@ Walk: // Incomplete match to end of edge if charsMatched < len(path) && charsMatchedInNodeFound == len(current.key) { - if current.catchAllKey != "" { - if !lazy { - *c.params = append(*c.params, Param{Key: current.catchAllKey, Value: path[charsMatched:]}) - // Same as exact match, no tsr recommendation - return current, false - } - // Same as exact match, no tsr recommendation - return current, false - } - // Tsr recommendation: remove the extra trailing slash (got an exact match) if !tsr { remainingKeySuffix := path[charsMatched:] @@ -787,30 +879,9 @@ Walk: Backtrack: if hasSkpNds { skipped := c.skipNds.pop() - if skipped.n.paramChildIndex < 0 || skipped.seen { - // skipped is catch all - current = skipped.n - *c.params = (*c.params)[:skipped.paramCnt] - - if !lazy { - *c.params = append(*c.params, Param{Key: current.catchAllKey, Value: path[skipped.pathIndex:]}) - // Same as exact match, no tsr recommendation - return current, false - } - // Same as exact match, no tsr recommendation - return current, false - } - - // Could be a catch-all node with child param - // /foo/*{any} - // /foo/{bar} - // In this case we evaluate first the child param node and fall back to the catch-all. - if skipped.n.catchAllKey != "" { - *c.skipNds = append(*c.skipNds, skippedNode{skipped.n, skipped.pathIndex, skipped.paramCnt, true}) - } parent = skipped.n - current = skipped.n.children[skipped.n.paramChildIndex].Load() + current = skipped.n.children[skipped.childIndex].Load() *c.params = (*c.params)[:skipped.paramCnt] charsMatched = skipped.pathIndex @@ -965,3 +1036,14 @@ func (t *Tree) newRoute(path string, handler HandlerFunc, opts ...PathOption) *R return rte } + +func copyParams(src, dst *Params) { + if cap(*src) > cap(*dst) { + // Grow dst to a least cap(src) + *dst = slices.Grow(*dst, cap(*src)) + } + // cap(dst) >= cap(src) + // now constraint into len(src) & cap(src) + *dst = (*dst)[:len(*src):cap(*src)] + copy(*dst, *src) +}