From 7df818ffff0958bb8821465d069a2ad130d448da Mon Sep 17 00:00:00 2001 From: Eduard Urbach Date: Sat, 2 Sep 2023 09:19:11 +0200 Subject: [PATCH 01/10] Reduced memory usage --- Parameter.go | 7 ----- README.md | 7 +++-- Router.go | 2 +- Router_test.go | 64 +++++++++++++++++++++++++++------------------- Tree.go | 36 +++----------------------- benchmarks_test.go | 49 +++++++++++++++++++++++------------ helper_test.go | 3 +++ keyValue.go | 7 +++++ testdata/blog.txt | 4 +++ 9 files changed, 95 insertions(+), 84 deletions(-) delete mode 100644 Parameter.go create mode 100644 keyValue.go create mode 100644 testdata/blog.txt diff --git a/Parameter.go b/Parameter.go deleted file mode 100644 index b1c7cd2..0000000 --- a/Parameter.go +++ /dev/null @@ -1,7 +0,0 @@ -package router - -// Parameter is a URL parameter. -type Parameter struct { - Key string - Value string -} diff --git a/README.md b/README.md index 9854c92..03d3956 100644 --- a/README.md +++ b/README.md @@ -54,8 +54,11 @@ coverage: 76.9% of statements ## Benchmarks ``` -BenchmarkLookup-12 6965749 171.2 ns/op 96 B/op 2 allocs/op -BenchmarkLookupNoAlloc-12 24243546 48.5 ns/op 0 B/op 0 allocs/op +BenchmarkBlog/Len1-Param0-12 182590244 6.57 ns/op 0 B/op 0 allocs/op +BenchmarkBlog/Len1-Param1-12 100000000 10.95 ns/op 0 B/op 0 allocs/op +BenchmarkGitHub/Len7-Param0-12 67053636 17.95 ns/op 0 B/op 0 allocs/op +BenchmarkGitHub/Len7-Param1-12 49371550 24.12 ns/op 0 B/op 0 allocs/op +BenchmarkGitHub/Len7-Param2-12 24562465 48.83 ns/op 0 B/op 0 allocs/op ``` ## License diff --git a/Router.go b/Router.go index bbed698..df0ae32 100644 --- a/Router.go +++ b/Router.go @@ -27,7 +27,7 @@ func (router *Router[T]) Add(method string, path string, handler T) { } // Lookup finds the handler and parameters for the given route. -func (router *Router[T]) Lookup(method string, path string) (T, []Parameter) { +func (router *Router[T]) Lookup(method string, path string) (T, []keyValue) { if method[0] == 'G' { return router.get.Lookup(path) } diff --git a/Router_test.go b/Router_test.go index e88afe6..2e192c1 100644 --- a/Router_test.go +++ b/Router_test.go @@ -8,35 +8,35 @@ import ( ) func TestStatic(t *testing.T) { - router := router.New[string]() - router.Add("GET", "/hello", "Hello") - router.Add("GET", "/world", "World") + r := router.New[string]() + r.Add("GET", "/hello", "Hello") + r.Add("GET", "/world", "World") - data, params := router.Lookup("GET", "/hello") + data, params := r.Lookup("GET", "/hello") assert.Equal(t, len(params), 0) assert.Equal(t, data, "Hello") - data, params = router.Lookup("GET", "/world") + data, params = r.Lookup("GET", "/world") assert.Equal(t, len(params), 0) assert.Equal(t, data, "World") - data, params = router.Lookup("GET", "/404") + data, params = r.Lookup("GET", "/404") assert.Equal(t, len(params), 0) assert.Equal(t, data, "") } func TestParameter(t *testing.T) { - router := router.New[string]() - router.Add("GET", "/blog/:slug", "Blog post") - router.Add("GET", "/blog/:slug/comments/:id", "Comment") + r := router.New[string]() + r.Add("GET", "/blog/:slug", "Blog post") + r.Add("GET", "/blog/:slug/comments/:id", "Comment") - data, params := router.Lookup("GET", "/blog/hello-world") + data, params := r.Lookup("GET", "/blog/hello-world") assert.Equal(t, len(params), 1) assert.Equal(t, params[0].Key, "slug") assert.Equal(t, params[0].Value, "hello-world") assert.Equal(t, data, "Blog post") - data, params = router.Lookup("GET", "/blog/hello-world/comments/123") + data, params = r.Lookup("GET", "/blog/hello-world/comments/123") assert.Equal(t, len(params), 2) assert.Equal(t, params[0].Key, "slug") assert.Equal(t, params[0].Value, "hello-world") @@ -46,29 +46,29 @@ func TestParameter(t *testing.T) { } func TestWildcard(t *testing.T) { - router := router.New[string]() - router.Add("GET", "/", "Front page") - router.Add("GET", "/:slug", "Blog post") - router.Add("GET", "/users/:id", "Parameter") - router.Add("GET", "/images/*path", "Wildcard") + r := router.New[string]() + r.Add("GET", "/", "Front page") + r.Add("GET", "/:slug", "Blog post") + r.Add("GET", "/users/:id", "Parameter") + r.Add("GET", "/images/*path", "Wildcard") - data, params := router.Lookup("GET", "/") + data, params := r.Lookup("GET", "/") assert.Equal(t, len(params), 0) assert.Equal(t, data, "Front page") - data, params = router.Lookup("GET", "/blog-post") + data, params = r.Lookup("GET", "/blog-post") assert.Equal(t, len(params), 1) assert.Equal(t, params[0].Key, "slug") assert.Equal(t, params[0].Value, "blog-post") assert.Equal(t, data, "Blog post") - data, params = router.Lookup("GET", "/users/42") + data, params = r.Lookup("GET", "/users/42") assert.Equal(t, len(params), 1) assert.Equal(t, params[0].Key, "id") assert.Equal(t, params[0].Value, "42") assert.Equal(t, data, "Parameter") - data, params = router.Lookup("GET", "/images/favicon/256.png") + data, params = r.Lookup("GET", "/images/favicon/256.png") assert.Equal(t, len(params), 1) assert.Equal(t, params[0].Key, "path") assert.Equal(t, params[0].Value, "favicon/256.png") @@ -76,7 +76,6 @@ func TestWildcard(t *testing.T) { } func TestMethods(t *testing.T) { - router := router.New[string]() methods := []string{ "GET", "POST", @@ -89,26 +88,39 @@ func TestMethods(t *testing.T) { "OPTIONS", } + r := router.New[string]() + for _, method := range methods { - router.Add(method, "/", method) + r.Add(method, "/", method) } for _, method := range methods { - data, _ := router.Lookup(method, "/") + data, _ := r.Lookup(method, "/") assert.Equal(t, data, method) } } func TestGitHub(t *testing.T) { - router := router.New[string]() routes := loadRoutes("testdata/github.txt") + r := router.New[string]() for _, route := range routes { - router.Add(route.method, route.path, "octocat") + r.Add(route.method, route.path, "octocat") } for _, route := range routes { - data, _ := router.Lookup(route.method, route.path) + data, _ := r.Lookup(route.method, route.path) assert.Equal(t, data, "octocat") } } + +func TestMemoryUsage(t *testing.T) { + escape := func(a any) {} + + result := testing.Benchmark(func(b *testing.B) { + r := router.New[string]() + escape(r) + }) + + t.Logf("%d bytes", result.MemBytes) +} diff --git a/Tree.go b/Tree.go index d091e31..9c3553b 100644 --- a/Tree.go +++ b/Tree.go @@ -1,9 +1,5 @@ package router -import ( - "strings" -) - // controlFlow tells the main loop what it should do next. type controlFlow int @@ -16,23 +12,11 @@ const ( // Tree represents a radix tree. type Tree[T any] struct { - static map[string]T - root treeNode[T] - canBeStatic [2048]bool + root treeNode[T] } // Add adds a new element to the tree. func (tree *Tree[T]) Add(path string, data T) { - if !strings.Contains(path, ":") && !strings.Contains(path, "*") { - if tree.static == nil { - tree.static = map[string]T{} - } - - tree.static[path] = data - tree.canBeStatic[len(path)] = true - return - } - // Search tree for equal parts until we can no longer proceed i := 0 offset := 0 @@ -114,11 +98,11 @@ func (tree *Tree[T]) Add(path string, data T) { } // Lookup finds the data for the given path. -func (tree *Tree[T]) Lookup(path string) (T, []Parameter) { - var params []Parameter +func (tree *Tree[T]) Lookup(path string) (T, []keyValue) { + var params []keyValue data := tree.LookupNoAlloc(path, func(key string, value string) { - params = append(params, Parameter{key, value}) + params = append(params, keyValue{key, value}) }) return data, params @@ -126,14 +110,6 @@ func (tree *Tree[T]) Lookup(path string) (T, []Parameter) { // LookupNoAlloc finds the data for the given path without using any memory allocations. func (tree *Tree[T]) LookupNoAlloc(path string, addParameter func(key string, value string)) T { - if tree.canBeStatic[len(path)] { - handler, found := tree.static[path] - - if found { - return handler - } - } - var ( i uint offset uint @@ -241,8 +217,4 @@ func (tree *Tree[T]) Bind(transform func(T) T) { tree.root.each(func(node *treeNode[T]) { node.data = transform(node.data) }) - - for key, value := range tree.static { - tree.static[key] = transform(value) - } } diff --git a/benchmarks_test.go b/benchmarks_test.go index 5182c3b..d72791e 100644 --- a/benchmarks_test.go +++ b/benchmarks_test.go @@ -6,33 +6,50 @@ import ( "git.akyoto.dev/go/router" ) -func BenchmarkLookup(b *testing.B) { - router := router.New[string]() - routes := loadRoutes("testdata/github.txt") +func BenchmarkBlog(b *testing.B) { + routes := loadRoutes("testdata/blog.txt") + r := router.New[string]() for _, route := range routes { - router.Add(route.method, route.path, "") + r.Add(route.method, route.path, "") } - b.ResetTimer() + b.Run("Len1-Param0", func(b *testing.B) { + for i := 0; i < b.N; i++ { + r.LookupNoAlloc("GET", "/", noop) + } + }) - for i := 0; i < b.N; i++ { - router.Lookup("GET", "/repos/:owner/:repo/issues") - } + b.Run("Len1-Param1", func(b *testing.B) { + for i := 0; i < b.N; i++ { + r.LookupNoAlloc("GET", "/:id", noop) + } + }) } -func BenchmarkLookupNoAlloc(b *testing.B) { - router := router.New[string]() +func BenchmarkGitHub(b *testing.B) { routes := loadRoutes("testdata/github.txt") - addParameter := func(string, string) {} + r := router.New[string]() for _, route := range routes { - router.Add(route.method, route.path, "") + r.Add(route.method, route.path, "") } - b.ResetTimer() + b.Run("Len7-Param0", func(b *testing.B) { + for i := 0; i < b.N; i++ { + r.LookupNoAlloc("GET", "/issues", noop) + } + }) - for i := 0; i < b.N; i++ { - router.LookupNoAlloc("GET", "/repos/:owner/:repo/issues", addParameter) - } + b.Run("Len7-Param1", func(b *testing.B) { + for i := 0; i < b.N; i++ { + r.LookupNoAlloc("GET", "/gists/:id", noop) + } + }) + + b.Run("Len7-Param2", func(b *testing.B) { + for i := 0; i < b.N; i++ { + r.LookupNoAlloc("GET", "/repos/:owner/:repo/issues", noop) + } + }) } diff --git a/helper_test.go b/helper_test.go index 1ad5d3c..92b14b1 100644 --- a/helper_test.go +++ b/helper_test.go @@ -50,3 +50,6 @@ func linesInFile(fileName string) <-chan string { return lines } + +// noop serves as an empty addParameter function. +func noop(string, string) {} diff --git a/keyValue.go b/keyValue.go new file mode 100644 index 0000000..def3969 --- /dev/null +++ b/keyValue.go @@ -0,0 +1,7 @@ +package router + +// keyValue represents a URL parameter. +type keyValue struct { + Key string + Value string +} diff --git a/testdata/blog.txt b/testdata/blog.txt new file mode 100644 index 0000000..432f70c --- /dev/null +++ b/testdata/blog.txt @@ -0,0 +1,4 @@ +GET / +GET /:slug +GET /tags +GET /tag/:tag From 763686679fdcc06574c756e466310e9e80dd50d0 Mon Sep 17 00:00:00 2001 From: Eduard Urbach Date: Wed, 6 Sep 2023 11:44:04 +0200 Subject: [PATCH 02/10] Improved test coverage --- README.md | 3 ++- Router_test.go | 62 ++++++++++++++++++++++++++++++++++++++++++-------- Tree.go | 5 ++++ 3 files changed, 60 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 03d3956..b955ed4 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,8 @@ PASS: TestParameter PASS: TestWildcard PASS: TestMethods PASS: TestGitHub -coverage: 76.9% of statements +PASS: TestOverwrite +coverage: 82.5% of statements ``` ## Benchmarks diff --git a/Router_test.go b/Router_test.go index 2e192c1..355c563 100644 --- a/Router_test.go +++ b/Router_test.go @@ -20,25 +20,36 @@ func TestStatic(t *testing.T) { assert.Equal(t, len(params), 0) assert.Equal(t, data, "World") - data, params = r.Lookup("GET", "/404") - assert.Equal(t, len(params), 0) - assert.Equal(t, data, "") + notFound := []string{ + "", + "?", + "/404", + "/hell", + "/hall", + "/helloo", + } + + for _, path := range notFound { + data, params = r.Lookup("GET", path) + assert.Equal(t, len(params), 0) + assert.Equal(t, data, "") + } } func TestParameter(t *testing.T) { r := router.New[string]() - r.Add("GET", "/blog/:slug", "Blog post") - r.Add("GET", "/blog/:slug/comments/:id", "Comment") + r.Add("GET", "/blog/:post", "Blog post") + r.Add("GET", "/blog/:post/comments/:id", "Comment") data, params := r.Lookup("GET", "/blog/hello-world") assert.Equal(t, len(params), 1) - assert.Equal(t, params[0].Key, "slug") + assert.Equal(t, params[0].Key, "post") assert.Equal(t, params[0].Value, "hello-world") assert.Equal(t, data, "Blog post") data, params = r.Lookup("GET", "/blog/hello-world/comments/123") assert.Equal(t, len(params), 2) - assert.Equal(t, params[0].Key, "slug") + assert.Equal(t, params[0].Key, "post") assert.Equal(t, params[0].Value, "hello-world") assert.Equal(t, params[1].Key, "id") assert.Equal(t, params[1].Value, "123") @@ -48,8 +59,9 @@ func TestParameter(t *testing.T) { func TestWildcard(t *testing.T) { r := router.New[string]() r.Add("GET", "/", "Front page") - r.Add("GET", "/:slug", "Blog post") + r.Add("GET", "/:post", "Blog post") r.Add("GET", "/users/:id", "Parameter") + r.Add("GET", "/images/static", "Static") r.Add("GET", "/images/*path", "Wildcard") data, params := r.Lookup("GET", "/") @@ -58,7 +70,7 @@ func TestWildcard(t *testing.T) { data, params = r.Lookup("GET", "/blog-post") assert.Equal(t, len(params), 1) - assert.Equal(t, params[0].Key, "slug") + assert.Equal(t, params[0].Key, "post") assert.Equal(t, params[0].Value, "blog-post") assert.Equal(t, data, "Blog post") @@ -68,6 +80,22 @@ func TestWildcard(t *testing.T) { assert.Equal(t, params[0].Value, "42") assert.Equal(t, data, "Parameter") + data, params = r.Lookup("GET", "/images/static") + assert.Equal(t, len(params), 0) + assert.Equal(t, data, "Static") + + data, params = r.Lookup("GET", "/images/ste") + assert.Equal(t, len(params), 1) + assert.Equal(t, params[0].Key, "path") + assert.Equal(t, params[0].Value, "ste") + assert.Equal(t, data, "Wildcard") + + data, params = r.Lookup("GET", "/images/sta") + assert.Equal(t, len(params), 1) + assert.Equal(t, params[0].Key, "path") + assert.Equal(t, params[0].Value, "sta") + assert.Equal(t, data, "Wildcard") + data, params = r.Lookup("GET", "/images/favicon/256.png") assert.Equal(t, len(params), 1) assert.Equal(t, params[0].Key, "path") @@ -111,9 +139,25 @@ func TestGitHub(t *testing.T) { for _, route := range routes { data, _ := r.Lookup(route.method, route.path) assert.Equal(t, data, "octocat") + + data = r.LookupNoAlloc(route.method, route.path, func(string, string) {}) + assert.Equal(t, data, "octocat") } } +func TestOverwrite(t *testing.T) { + r := router.New[string]() + r.Add("GET", "/", "1") + r.Add("GET", "/", "2") + r.Add("GET", "/", "3") + r.Add("GET", "/", "4") + r.Add("GET", "/", "5") + + data, params := r.Lookup("GET", "/") + assert.Equal(t, len(params), 0) + assert.Equal(t, data, "5") +} + func TestMemoryUsage(t *testing.T) { escape := func(a any) {} diff --git a/Tree.go b/Tree.go index 9c3553b..3f150a3 100644 --- a/Tree.go +++ b/Tree.go @@ -130,6 +130,11 @@ begin: return node.data } + if lastWildcard != nil { + addParameter(lastWildcard.prefix, path[lastWildcardOffset:]) + return lastWildcard.data + } + // node: /blog|feed // path: /blog| return empty From 0323126f2c0b36956268a9e9f2673a84009395c9 Mon Sep 17 00:00:00 2001 From: Eduard Urbach Date: Wed, 6 Sep 2023 16:52:22 +0200 Subject: [PATCH 03/10] Improved test coverage --- README.md | 5 ++++- Router.go | 30 ++++++++++---------------- Router_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++ Tree.go | 12 ++++++----- treeNode.go | 46 --------------------------------------- 5 files changed, 80 insertions(+), 71 deletions(-) diff --git a/README.md b/README.md index b955ed4..382017a 100644 --- a/README.md +++ b/README.md @@ -46,10 +46,13 @@ data := router.LookupNoAlloc("GET", "/users/42", func(key string, value string) PASS: TestStatic PASS: TestParameter PASS: TestWildcard +PASS: TestMap PASS: TestMethods PASS: TestGitHub +PASS: TestTrailingSlash PASS: TestOverwrite -coverage: 82.5% of statements +PASS: TestInvalidMethod +coverage: 99.1% of statements ``` ## Benchmarks diff --git a/Router.go b/Router.go index df0ae32..d3116a5 100644 --- a/Router.go +++ b/Router.go @@ -1,7 +1,5 @@ package router -import "os" - // Router is a high-performance router. type Router[T any] struct { get Tree[T] @@ -46,23 +44,17 @@ func (router *Router[T]) LookupNoAlloc(method string, path string, addParameter return tree.LookupNoAlloc(path, addParameter) } -// Bind traverses all trees and calls the given function on every node. -func (router *Router[T]) Bind(transform func(T) T) { - router.get.Bind(transform) - router.post.Bind(transform) - router.delete.Bind(transform) - router.put.Bind(transform) - router.patch.Bind(transform) - router.head.Bind(transform) - router.connect.Bind(transform) - router.trace.Bind(transform) - router.options.Bind(transform) -} - -// Print shows a pretty print of all the routes. -func (router *Router[T]) Print(method string) { - tree := router.selectTree(method) - tree.root.PrettyPrint(os.Stdout) +// Map traverses all trees and calls the given function on every node. +func (router *Router[T]) Map(transform func(T) T) { + router.get.Map(transform) + router.post.Map(transform) + router.delete.Map(transform) + router.put.Map(transform) + router.patch.Map(transform) + router.head.Map(transform) + router.connect.Map(transform) + router.trace.Map(transform) + router.options.Map(transform) } // selectTree returns the tree by the given HTTP method. diff --git a/Router_test.go b/Router_test.go index 355c563..cc93b46 100644 --- a/Router_test.go +++ b/Router_test.go @@ -1,6 +1,7 @@ package router_test import ( + "strings" "testing" "git.akyoto.dev/go/assert" @@ -60,6 +61,7 @@ func TestWildcard(t *testing.T) { r := router.New[string]() r.Add("GET", "/", "Front page") r.Add("GET", "/:post", "Blog post") + r.Add("GET", "/*any", "Wildcard") r.Add("GET", "/users/:id", "Parameter") r.Add("GET", "/images/static", "Static") r.Add("GET", "/images/*path", "Wildcard") @@ -80,6 +82,9 @@ func TestWildcard(t *testing.T) { assert.Equal(t, params[0].Value, "42") assert.Equal(t, data, "Parameter") + data, _ = r.Lookup("GET", "/users/42/test.txt") + assert.Equal(t, data, "Wildcard") + data, params = r.Lookup("GET", "/images/static") assert.Equal(t, len(params), 0) assert.Equal(t, data, "Static") @@ -103,6 +108,34 @@ func TestWildcard(t *testing.T) { assert.Equal(t, data, "Wildcard") } +func TestMap(t *testing.T) { + r := router.New[string]() + r.Add("GET", "/hello", "Hello") + r.Add("GET", "/world", "World") + r.Add("GET", "/user/:user", "User") + r.Add("GET", "/*path", "Path") + + r.Map(func(data string) string { + return strings.Repeat(data, 2) + }) + + data, params := r.Lookup("GET", "/hello") + assert.Equal(t, len(params), 0) + assert.Equal(t, data, "HelloHello") + + data, params = r.Lookup("GET", "/world") + assert.Equal(t, len(params), 0) + assert.Equal(t, data, "WorldWorld") + + data, params = r.Lookup("GET", "/user/123") + assert.Equal(t, len(params), 1) + assert.Equal(t, data, "UserUser") + + data, params = r.Lookup("GET", "/test.txt") + assert.Equal(t, len(params), 1) + assert.Equal(t, data, "PathPath") +} + func TestMethods(t *testing.T) { methods := []string{ "GET", @@ -145,6 +178,20 @@ func TestGitHub(t *testing.T) { } } +func TestTrailingSlash(t *testing.T) { + r := router.New[string]() + r.Add("GET", "/hello", "Hello 1") + r.Add("GET", "/hello/", "Hello 2") + + data, params := r.Lookup("GET", "/hello") + assert.Equal(t, len(params), 0) + assert.Equal(t, data, "Hello 1") + + data, params = r.Lookup("GET", "/hello/") + assert.Equal(t, len(params), 0) + assert.Equal(t, data, "Hello 2") +} + func TestOverwrite(t *testing.T) { r := router.New[string]() r.Add("GET", "/", "1") @@ -158,6 +205,17 @@ func TestOverwrite(t *testing.T) { assert.Equal(t, data, "5") } +func TestInvalidMethod(t *testing.T) { + defer func() { + if recover() == nil { + t.FailNow() + } + }() + + r := router.New[string]() + r.Add("?", "/hello", "Hello") +} + func TestMemoryUsage(t *testing.T) { escape := func(a any) {} diff --git a/Tree.go b/Tree.go index 3f150a3..3193d8a 100644 --- a/Tree.go +++ b/Tree.go @@ -130,6 +130,8 @@ begin: return node.data } + // node: /|*any + // path: /|image.png if lastWildcard != nil { addParameter(lastWildcard.prefix, path[lastWildcardOffset:]) return lastWildcard.data @@ -193,9 +195,9 @@ begin: // node: /|*any // path: /|image.png - if node.wildcard != nil { - addParameter(node.wildcard.prefix, path[i:]) - return node.wildcard.data + if lastWildcard != nil { + addParameter(lastWildcard.prefix, path[lastWildcardOffset:]) + return lastWildcard.data } return empty @@ -217,8 +219,8 @@ begin: } } -// Bind binds all handlers to a new one provided by the callback. -func (tree *Tree[T]) Bind(transform func(T) T) { +// Map binds all handlers to a new one provided by the callback. +func (tree *Tree[T]) Map(transform func(T) T) { tree.root.each(func(node *treeNode[T]) { node.data = transform(node.data) }) diff --git a/treeNode.go b/treeNode.go index 18af73c..ab430a6 100644 --- a/treeNode.go +++ b/treeNode.go @@ -1,8 +1,6 @@ package router import ( - "fmt" - "io" "strings" ) @@ -283,47 +281,3 @@ func (node *treeNode[T]) each(callback func(*treeNode[T])) { node.wildcard.each(callback) } } - -// PrettyPrint prints a human-readable form of the tree to the given writer. -func (node *treeNode[T]) PrettyPrint(writer io.Writer) { - node.prettyPrint(writer, -1) -} - -// prettyPrint is the underlying pretty printer. -func (node *treeNode[T]) prettyPrint(writer io.Writer, level int) { - prefix := "" - - if level >= 0 { - prefix = strings.Repeat(" ", level) + "|_ " - } - - switch node.kind { - case ':': - prefix += ":" - case '*': - prefix += "*" - } - - fmt.Fprintf(writer, "%s%s [%v]\n", prefix, node.prefix, node.data) - - for _, child := range node.children { - if child == nil { - continue - } - - child.prettyPrint(writer, level+1) - } - - if node.parameter != nil { - node.parameter.prettyPrint(writer, level+1) - } - - if node.wildcard != nil { - node.wildcard.prettyPrint(writer, level+1) - } -} - -// String returns the node prefix. -func (node *treeNode[T]) String() string { - return node.prefix -} From a7d64037a70cffa2ccd89f7676b072e23a383b68 Mon Sep 17 00:00:00 2001 From: Eduard Urbach Date: Wed, 13 Mar 2024 13:04:03 +0100 Subject: [PATCH 04/10] Added trailing slash for static routes --- README.md | 3 ++- Router_test.go | 40 ++++++++++++++++++++++++++++----- Tree.go | 31 +++++--------------------- benchmarks_test.go | 12 ++++++---- flow.go | 11 ++++++++++ helper_test.go | 55 ---------------------------------------------- testdata/Route.go | 52 +++++++++++++++++++++++++++++++++++++++++++ treeNode.go | 11 +++++----- 8 files changed, 119 insertions(+), 96 deletions(-) create mode 100644 flow.go delete mode 100644 helper_test.go create mode 100644 testdata/Route.go diff --git a/README.md b/README.md index 382017a..724d820 100644 --- a/README.md +++ b/README.md @@ -50,9 +50,10 @@ PASS: TestMap PASS: TestMethods PASS: TestGitHub PASS: TestTrailingSlash +PASS: TestTrailingSlashOverwrite PASS: TestOverwrite PASS: TestInvalidMethod -coverage: 99.1% of statements +coverage: 100.0% of statements ``` ## Benchmarks diff --git a/Router_test.go b/Router_test.go index cc93b46..66230c6 100644 --- a/Router_test.go +++ b/Router_test.go @@ -6,6 +6,7 @@ import ( "git.akyoto.dev/go/assert" "git.akyoto.dev/go/router" + "git.akyoto.dev/go/router/testdata" ) func TestStatic(t *testing.T) { @@ -162,18 +163,18 @@ func TestMethods(t *testing.T) { } func TestGitHub(t *testing.T) { - routes := loadRoutes("testdata/github.txt") + routes := testdata.Routes("testdata/github.txt") r := router.New[string]() for _, route := range routes { - r.Add(route.method, route.path, "octocat") + r.Add(route.Method, route.Path, "octocat") } for _, route := range routes { - data, _ := r.Lookup(route.method, route.path) + data, _ := r.Lookup(route.Method, route.Path) assert.Equal(t, data, "octocat") - data = r.LookupNoAlloc(route.method, route.path, func(string, string) {}) + data = r.LookupNoAlloc(route.Method, route.Path, func(string, string) {}) assert.Equal(t, data, "octocat") } } @@ -181,7 +182,6 @@ func TestGitHub(t *testing.T) { func TestTrailingSlash(t *testing.T) { r := router.New[string]() r.Add("GET", "/hello", "Hello 1") - r.Add("GET", "/hello/", "Hello 2") data, params := r.Lookup("GET", "/hello") assert.Equal(t, len(params), 0) @@ -189,7 +189,35 @@ func TestTrailingSlash(t *testing.T) { data, params = r.Lookup("GET", "/hello/") assert.Equal(t, len(params), 0) - assert.Equal(t, data, "Hello 2") + assert.Equal(t, data, "Hello 1") +} + +func TestTrailingSlashOverwrite(t *testing.T) { + r := router.New[string]() + r.Add("GET", "/hello", "route 1") + r.Add("GET", "/hello/", "route 2") + r.Add("GET", "/:param", "route 3") + r.Add("GET", "/:param/", "route 4") + r.Add("GET", "/*any", "route 5") + + data, params := r.Lookup("GET", "/hello") + assert.Equal(t, len(params), 0) + assert.Equal(t, data, "route 1") + + data, params = r.Lookup("GET", "/hello/") + assert.Equal(t, len(params), 0) + assert.Equal(t, data, "route 2") + + data, params = r.Lookup("GET", "/param") + assert.Equal(t, len(params), 1) + assert.Equal(t, data, "route 3") + + data, params = r.Lookup("GET", "/param/") + assert.Equal(t, len(params), 1) + assert.Equal(t, data, "route 4") + + data, _ = r.Lookup("GET", "/wild/card/") + assert.Equal(t, data, "route 5") } func TestOverwrite(t *testing.T) { diff --git a/Tree.go b/Tree.go index 3193d8a..5dfe82b 100644 --- a/Tree.go +++ b/Tree.go @@ -1,15 +1,5 @@ package router -// controlFlow tells the main loop what it should do next. -type controlFlow int - -// controlFlow values. -const ( - controlStop controlFlow = 0 - controlBegin controlFlow = 1 - controlNext controlFlow = 2 -) - // Tree represents a radix tree. type Tree[T any] struct { root treeNode[T] @@ -36,17 +26,8 @@ func (tree *Tree[T]) Add(path string, data T) { // When we hit a separator, we'll search for a fitting child. if path[i] == separator { - var control controlFlow - node, offset, control = node.end(path, data, i, offset) - - switch control { - case controlStop: - return - case controlBegin: - goto begin - case controlNext: - goto next - } + node, offset, _ = node.end(path, data, i, offset) + goto next } default: @@ -70,15 +51,15 @@ func (tree *Tree[T]) Add(path string, data T) { // node: /| // path: /|blog if i-offset == len(node.prefix) { - var control controlFlow + var control flow node, offset, control = node.end(path, data, i, offset) switch control { - case controlStop: + case flowStop: return - case controlBegin: + case flowBegin: goto begin - case controlNext: + case flowNext: goto next } } diff --git a/benchmarks_test.go b/benchmarks_test.go index d72791e..55caf1f 100644 --- a/benchmarks_test.go +++ b/benchmarks_test.go @@ -4,14 +4,15 @@ import ( "testing" "git.akyoto.dev/go/router" + "git.akyoto.dev/go/router/testdata" ) func BenchmarkBlog(b *testing.B) { - routes := loadRoutes("testdata/blog.txt") + routes := testdata.Routes("testdata/blog.txt") r := router.New[string]() for _, route := range routes { - r.Add(route.method, route.path, "") + r.Add(route.Method, route.Path, "") } b.Run("Len1-Param0", func(b *testing.B) { @@ -28,11 +29,11 @@ func BenchmarkBlog(b *testing.B) { } func BenchmarkGitHub(b *testing.B) { - routes := loadRoutes("testdata/github.txt") + routes := testdata.Routes("testdata/github.txt") r := router.New[string]() for _, route := range routes { - r.Add(route.method, route.path, "") + r.Add(route.Method, route.Path, "") } b.Run("Len7-Param0", func(b *testing.B) { @@ -53,3 +54,6 @@ func BenchmarkGitHub(b *testing.B) { } }) } + +// noop serves as an empty addParameter function. +func noop(string, string) {} diff --git a/flow.go b/flow.go new file mode 100644 index 0000000..d84bcaa --- /dev/null +++ b/flow.go @@ -0,0 +1,11 @@ +package router + +// flow tells the main loop what it should do next. +type flow int + +// Control flow values. +const ( + flowStop flow = iota + flowBegin + flowNext +) diff --git a/helper_test.go b/helper_test.go deleted file mode 100644 index 92b14b1..0000000 --- a/helper_test.go +++ /dev/null @@ -1,55 +0,0 @@ -package router_test - -import ( - "bufio" - "os" - "strings" -) - -// route represents a single line in the router test file. -type route struct { - method string - path string -} - -// loadRoutes loads all routes from a text file. -func loadRoutes(fileName string) []route { - var routes []route - - for line := range linesInFile(fileName) { - line = strings.TrimSpace(line) - parts := strings.Split(line, " ") - routes = append(routes, route{ - method: parts[0], - path: parts[1], - }) - } - - return routes -} - -// linesInFile is a utility function to easily read every line in a text file. -func linesInFile(fileName string) <-chan string { - lines := make(chan string) - - go func() { - defer close(lines) - file, err := os.Open(fileName) - - if err != nil { - return - } - - defer file.Close() - scanner := bufio.NewScanner(file) - - for scanner.Scan() { - lines <- scanner.Text() - } - }() - - return lines -} - -// noop serves as an empty addParameter function. -func noop(string, string) {} diff --git a/testdata/Route.go b/testdata/Route.go new file mode 100644 index 0000000..2a2231f --- /dev/null +++ b/testdata/Route.go @@ -0,0 +1,52 @@ +package testdata + +import ( + "bufio" + "os" + "strings" +) + +// Route represents a single line in the router test file. +type Route struct { + Method string + Path string +} + +// Routes loads all routes from a text file. +func Routes(fileName string) []Route { + var routes []Route + + for line := range Lines(fileName) { + line = strings.TrimSpace(line) + parts := strings.Split(line, " ") + routes = append(routes, Route{ + Method: parts[0], + Path: parts[1], + }) + } + + return routes +} + +// Lines is a utility function to easily read every line in a text file. +func Lines(fileName string) <-chan string { + lines := make(chan string) + + go func() { + defer close(lines) + file, err := os.Open(fileName) + + if err != nil { + return + } + + defer file.Close() + scanner := bufio.NewScanner(file) + + for scanner.Scan() { + lines <- scanner.Text() + } + }() + + return lines +} diff --git a/treeNode.go b/treeNode.go index ab430a6..c500939 100644 --- a/treeNode.go +++ b/treeNode.go @@ -157,6 +157,7 @@ func (node *treeNode[T]) append(path string, data T) { if node.prefix == "" { node.prefix = path node.data = data + node.addTrailingSlash(data) return } @@ -229,7 +230,7 @@ func (node *treeNode[T]) append(path string, data T) { // end is called when the node was fully parsed // and needs to decide the next control flow. // end is only called from `tree.Add`. -func (node *treeNode[T]) end(path string, data T, i int, offset int) (*treeNode[T], int, controlFlow) { +func (node *treeNode[T]) end(path string, data T, i int, offset int) (*treeNode[T], int, flow) { char := path[i] if char >= node.startIndex && char < node.endIndex { @@ -238,7 +239,7 @@ func (node *treeNode[T]) end(path string, data T, i int, offset int) (*treeNode[ if index != 0 { node = node.children[index] offset = i - return node, offset, controlNext + return node, offset, flowNext } } @@ -246,7 +247,7 @@ func (node *treeNode[T]) end(path string, data T, i int, offset int) (*treeNode[ // If no prefix is set, this is the starting node. if node.prefix == "" { node.append(path[i:], data) - return node, offset, controlStop + return node, offset, flowStop } // node: /user/|:id @@ -254,11 +255,11 @@ func (node *treeNode[T]) end(path string, data T, i int, offset int) (*treeNode[ if node.parameter != nil && path[i] == parameter { node = node.parameter offset = i - return node, offset, controlBegin + return node, offset, flowBegin } node.append(path[i:], data) - return node, offset, controlStop + return node, offset, flowStop } // each traverses the tree and calls the given function on every node. From b04aadea0911536627d8bd15c6216354306defca Mon Sep 17 00:00:00 2001 From: Eduard Urbach Date: Thu, 14 Mar 2024 23:26:59 +0100 Subject: [PATCH 05/10] Improved performance --- Parameter.go | 7 +++++ README.md | 10 +++---- Router.go | 2 +- Router_test.go | 30 +++++++++++++++++-- Tree.go | 79 ++++++++++++++++++++++---------------------------- go.mod | 2 +- keyValue.go | 7 ----- 7 files changed, 77 insertions(+), 60 deletions(-) create mode 100644 Parameter.go delete mode 100644 keyValue.go diff --git a/Parameter.go b/Parameter.go new file mode 100644 index 0000000..de4f0a5 --- /dev/null +++ b/Parameter.go @@ -0,0 +1,7 @@ +package router + +// Parameter represents a URL parameter. +type Parameter struct { + Key string + Value string +} diff --git a/README.md b/README.md index 724d820..116f444 100644 --- a/README.md +++ b/README.md @@ -59,11 +59,11 @@ coverage: 100.0% of statements ## Benchmarks ``` -BenchmarkBlog/Len1-Param0-12 182590244 6.57 ns/op 0 B/op 0 allocs/op -BenchmarkBlog/Len1-Param1-12 100000000 10.95 ns/op 0 B/op 0 allocs/op -BenchmarkGitHub/Len7-Param0-12 67053636 17.95 ns/op 0 B/op 0 allocs/op -BenchmarkGitHub/Len7-Param1-12 49371550 24.12 ns/op 0 B/op 0 allocs/op -BenchmarkGitHub/Len7-Param2-12 24562465 48.83 ns/op 0 B/op 0 allocs/op +BenchmarkBlog/Len1-Param0-12 200621386 5.963 ns/op 0 B/op 0 allocs/op +BenchmarkBlog/Len1-Param1-12 129550375 9.224 ns/op 0 B/op 0 allocs/op +BenchmarkGitHub/Len7-Param0-12 70562060 16.98 ns/op 0 B/op 0 allocs/op +BenchmarkGitHub/Len7-Param1-12 49366180 22.56 ns/op 0 B/op 0 allocs/op +BenchmarkGitHub/Len7-Param2-12 24332162 47.91 ns/op 0 B/op 0 allocs/op ``` ## License diff --git a/Router.go b/Router.go index d3116a5..3db0647 100644 --- a/Router.go +++ b/Router.go @@ -25,7 +25,7 @@ func (router *Router[T]) Add(method string, path string, handler T) { } // Lookup finds the handler and parameters for the given route. -func (router *Router[T]) Lookup(method string, path string) (T, []keyValue) { +func (router *Router[T]) Lookup(method string, path string) (T, []Parameter) { if method[0] == 'G' { return router.get.Lookup(path) } diff --git a/Router_test.go b/Router_test.go index 66230c6..3f1c2fe 100644 --- a/Router_test.go +++ b/Router_test.go @@ -9,6 +9,20 @@ import ( "git.akyoto.dev/go/router/testdata" ) +func TestHello(t *testing.T) { + r := router.New[string]() + r.Add("GET", "/blog", "Blog") + r.Add("GET", "/blog/post", "Blog post") + + data, params := r.Lookup("GET", "/blog") + assert.Equal(t, len(params), 0) + assert.Equal(t, data, "Blog") + + data, params = r.Lookup("GET", "/blog/post") + assert.Equal(t, len(params), 0) + assert.Equal(t, data, "Blog post") +} + func TestStatic(t *testing.T) { r := router.New[string]() r.Add("GET", "/hello", "Hello") @@ -61,11 +75,12 @@ func TestParameter(t *testing.T) { func TestWildcard(t *testing.T) { r := router.New[string]() r.Add("GET", "/", "Front page") - r.Add("GET", "/:post", "Blog post") - r.Add("GET", "/*any", "Wildcard") r.Add("GET", "/users/:id", "Parameter") r.Add("GET", "/images/static", "Static") r.Add("GET", "/images/*path", "Wildcard") + r.Add("GET", "/:post", "Blog post") + r.Add("GET", "/*any", "Wildcard") + r.Add("GET", "*root", "Root wildcard") data, params := r.Lookup("GET", "/") assert.Equal(t, len(params), 0) @@ -107,6 +122,12 @@ func TestWildcard(t *testing.T) { assert.Equal(t, params[0].Key, "path") assert.Equal(t, params[0].Value, "favicon/256.png") assert.Equal(t, data, "Wildcard") + + data, params = r.Lookup("GET", "not-a-path") + assert.Equal(t, len(params), 1) + assert.Equal(t, params[0].Key, "root") + assert.Equal(t, params[0].Value, "not-a-path") + assert.Equal(t, data, "Root wildcard") } func TestMap(t *testing.T) { @@ -115,6 +136,7 @@ func TestMap(t *testing.T) { r.Add("GET", "/world", "World") r.Add("GET", "/user/:user", "User") r.Add("GET", "/*path", "Path") + r.Add("GET", "*root", "Root") r.Map(func(data string) string { return strings.Repeat(data, 2) @@ -135,6 +157,10 @@ func TestMap(t *testing.T) { data, params = r.Lookup("GET", "/test.txt") assert.Equal(t, len(params), 1) assert.Equal(t, data, "PathPath") + + data, params = r.Lookup("GET", "test.txt") + assert.Equal(t, len(params), 1) + assert.Equal(t, data, "RootRoot") } func TestMethods(t *testing.T) { diff --git a/Tree.go b/Tree.go index 5dfe82b..5f33073 100644 --- a/Tree.go +++ b/Tree.go @@ -79,11 +79,11 @@ func (tree *Tree[T]) Add(path string, data T) { } // Lookup finds the data for the given path. -func (tree *Tree[T]) Lookup(path string) (T, []keyValue) { - var params []keyValue +func (tree *Tree[T]) Lookup(path string) (T, []Parameter) { + var params []Parameter data := tree.LookupNoAlloc(path, func(key string, value string) { - params = append(params, keyValue{key, value}) + params = append(params, Parameter{key, value}) }) return data, params @@ -92,44 +92,28 @@ func (tree *Tree[T]) Lookup(path string) (T, []keyValue) { // LookupNoAlloc finds the data for the given path without using any memory allocations. func (tree *Tree[T]) LookupNoAlloc(path string, addParameter func(key string, value string)) T { var ( - i uint - offset uint - lastWildcardOffset uint - lastWildcard *treeNode[T] - empty T - node = &tree.root + i uint + offset uint + wildcardOffset uint + wildcard *treeNode[T] + node = &tree.root ) + // Skip the first loop iteration if the starting characters are equal + if len(path) > 0 && len(node.prefix) > 0 && path[0] == node.prefix[0] { + i = 1 + } + begin: // Search tree for equal parts until we can no longer proceed - for { - // We reached the end. - if i == uint(len(path)) { - // node: /blog| - // path: /blog| - if i-offset == uint(len(node.prefix)) { - return node.data - } - - // node: /|*any - // path: /|image.png - if lastWildcard != nil { - addParameter(lastWildcard.prefix, path[lastWildcardOffset:]) - return lastWildcard.data - } - - // node: /blog|feed - // path: /blog| - return empty - } - + for i < uint(len(path)) { // The node we just checked is entirely included in our path. // node: /| // path: /|blog if i-offset == uint(len(node.prefix)) { if node.wildcard != nil { - lastWildcard = node.wildcard - lastWildcardOffset = i + wildcard = node.wildcard + wildcardOffset = i } char := path[i] @@ -176,28 +160,35 @@ begin: // node: /|*any // path: /|image.png - if lastWildcard != nil { - addParameter(lastWildcard.prefix, path[lastWildcardOffset:]) - return lastWildcard.data - } - - return empty + goto notFound } // We got a conflict. // node: /b|ag // path: /b|riefcase if path[i] != node.prefix[i-offset] { - if lastWildcard != nil { - addParameter(lastWildcard.prefix, path[lastWildcardOffset:]) - return lastWildcard.data - } - - return empty + goto notFound } i++ } + + // node: /blog| + // path: /blog| + if i-offset == uint(len(node.prefix)) { + return node.data + } + + // node: /|*any + // path: /|image.png +notFound: + if wildcard != nil { + addParameter(wildcard.prefix, path[wildcardOffset:]) + return wildcard.data + } + + var empty T + return empty } // Map binds all handlers to a new one provided by the callback. diff --git a/go.mod b/go.mod index 5879d15..9f71fa8 100644 --- a/go.mod +++ b/go.mod @@ -1,5 +1,5 @@ module git.akyoto.dev/go/router -go 1.21 +go 1.22 require git.akyoto.dev/go/assert v0.1.3 diff --git a/keyValue.go b/keyValue.go deleted file mode 100644 index def3969..0000000 --- a/keyValue.go +++ /dev/null @@ -1,7 +0,0 @@ -package router - -// keyValue represents a URL parameter. -type keyValue struct { - Key string - Value string -} From e348332fa93ea3d40292bd9bfc059e2877d05601 Mon Sep 17 00:00:00 2001 From: Eduard Urbach Date: Fri, 15 Mar 2024 00:07:44 +0100 Subject: [PATCH 06/10] Improved performance --- README.md | 10 +++++----- Tree.go | 11 ++++------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 116f444..7f91fa2 100644 --- a/README.md +++ b/README.md @@ -59,11 +59,11 @@ coverage: 100.0% of statements ## Benchmarks ``` -BenchmarkBlog/Len1-Param0-12 200621386 5.963 ns/op 0 B/op 0 allocs/op -BenchmarkBlog/Len1-Param1-12 129550375 9.224 ns/op 0 B/op 0 allocs/op -BenchmarkGitHub/Len7-Param0-12 70562060 16.98 ns/op 0 B/op 0 allocs/op -BenchmarkGitHub/Len7-Param1-12 49366180 22.56 ns/op 0 B/op 0 allocs/op -BenchmarkGitHub/Len7-Param2-12 24332162 47.91 ns/op 0 B/op 0 allocs/op +BenchmarkBlog/Len1-Param0-12 201446469 5.956 ns/op 0 B/op 0 allocs/op +BenchmarkBlog/Len1-Param1-12 130212613 9.221 ns/op 0 B/op 0 allocs/op +BenchmarkGitHub/Len7-Param0-12 70604431 16.97 ns/op 0 B/op 0 allocs/op +BenchmarkGitHub/Len7-Param1-12 50499285 22.55 ns/op 0 B/op 0 allocs/op +BenchmarkGitHub/Len7-Param2-12 25460875 46.84 ns/op 0 B/op 0 allocs/op ``` ## License diff --git a/Tree.go b/Tree.go index 5f33073..fef2ef6 100644 --- a/Tree.go +++ b/Tree.go @@ -136,13 +136,7 @@ begin: offset = i i++ - for { - // We reached the end. - if i == uint(len(path)) { - addParameter(node.prefix, path[offset:i]) - return node.data - } - + for i < uint(len(path)) { // node: /:id|/posts // path: /123|/posts if path[i] == separator { @@ -156,6 +150,9 @@ begin: i++ } + + addParameter(node.prefix, path[offset:i]) + return node.data } // node: /|*any From f11c90da3421097d9f9b650fbf04ed4374faaf6c Mon Sep 17 00:00:00 2001 From: Eduard Urbach Date: Fri, 15 Mar 2024 09:52:34 +0100 Subject: [PATCH 07/10] Improved performance --- README.md | 10 +++++----- Tree.go | 35 +++++++++++++++++------------------ 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 7f91fa2..01725fd 100644 --- a/README.md +++ b/README.md @@ -59,11 +59,11 @@ coverage: 100.0% of statements ## Benchmarks ``` -BenchmarkBlog/Len1-Param0-12 201446469 5.956 ns/op 0 B/op 0 allocs/op -BenchmarkBlog/Len1-Param1-12 130212613 9.221 ns/op 0 B/op 0 allocs/op -BenchmarkGitHub/Len7-Param0-12 70604431 16.97 ns/op 0 B/op 0 allocs/op -BenchmarkGitHub/Len7-Param1-12 50499285 22.55 ns/op 0 B/op 0 allocs/op -BenchmarkGitHub/Len7-Param2-12 25460875 46.84 ns/op 0 B/op 0 allocs/op +BenchmarkBlog/Len1-Param0-12 211814850 5.646 ns/op 0 B/op 0 allocs/op +BenchmarkBlog/Len1-Param1-12 132838722 8.978 ns/op 0 B/op 0 allocs/op +BenchmarkGitHub/Len7-Param0-12 84768382 14.14 ns/op 0 B/op 0 allocs/op +BenchmarkGitHub/Len7-Param1-12 55290044 20.74 ns/op 0 B/op 0 allocs/op +BenchmarkGitHub/Len7-Param2-12 26057244 46.08 ns/op 0 B/op 0 allocs/op ``` ## License diff --git a/Tree.go b/Tree.go index fef2ef6..e1f6bb0 100644 --- a/Tree.go +++ b/Tree.go @@ -92,11 +92,10 @@ func (tree *Tree[T]) Lookup(path string) (T, []Parameter) { // LookupNoAlloc finds the data for the given path without using any memory allocations. func (tree *Tree[T]) LookupNoAlloc(path string, addParameter func(key string, value string)) T { var ( - i uint - offset uint - wildcardOffset uint - wildcard *treeNode[T] - node = &tree.root + i uint + wildcardPath string + wildcard *treeNode[T] + node = &tree.root ) // Skip the first loop iteration if the starting characters are equal @@ -110,10 +109,10 @@ begin: // The node we just checked is entirely included in our path. // node: /| // path: /|blog - if i-offset == uint(len(node.prefix)) { + if i == uint(len(node.prefix)) { if node.wildcard != nil { wildcard = node.wildcard - wildcardOffset = i + wildcardPath = path[i:] } char := path[i] @@ -123,8 +122,8 @@ begin: if index != 0 { node = node.children[index] - offset = i - i++ + path = path[i:] + i = 1 continue } } @@ -133,25 +132,25 @@ begin: // path: /|blog if node.parameter != nil { node = node.parameter - offset = i - i++ + path = path[i:] + i = 1 for i < uint(len(path)) { // node: /:id|/posts // path: /123|/posts if path[i] == separator { - addParameter(node.prefix, path[offset:i]) + addParameter(node.prefix, path[:i]) index := node.indices[separator-node.startIndex] node = node.children[index] - offset = i - i++ + path = path[i:] + i = 1 goto begin } i++ } - addParameter(node.prefix, path[offset:i]) + addParameter(node.prefix, path[:i]) return node.data } @@ -163,7 +162,7 @@ begin: // We got a conflict. // node: /b|ag // path: /b|riefcase - if path[i] != node.prefix[i-offset] { + if path[i] != node.prefix[i] { goto notFound } @@ -172,7 +171,7 @@ begin: // node: /blog| // path: /blog| - if i-offset == uint(len(node.prefix)) { + if i == uint(len(node.prefix)) { return node.data } @@ -180,7 +179,7 @@ begin: // path: /|image.png notFound: if wildcard != nil { - addParameter(wildcard.prefix, path[wildcardOffset:]) + addParameter(wildcard.prefix, wildcardPath) return wildcard.data } From 7ba55e445e4c657a6cf6345b3003c68a970d9c7b Mon Sep 17 00:00:00 2001 From: Eduard Urbach Date: Tue, 25 Feb 2025 16:38:39 +0100 Subject: [PATCH 08/10] Updated module path --- README.md | 4 ++-- Router_test.go | 6 +++--- benchmarks_test.go | 4 ++-- go.mod | 6 +++--- go.sum | 4 ++-- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 01725fd..6dd27c4 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ HTTP router based on radix trees. ## Installation ```shell -go get git.akyoto.dev/go/router +go get git.urbach.dev/go/router ``` ## Usage @@ -68,7 +68,7 @@ BenchmarkGitHub/Len7-Param2-12 26057244 46.08 ns/op ## License -Please see the [license documentation](https://akyoto.dev/license). +Please see the [license documentation](https://urbach.dev/license). ## Copyright diff --git a/Router_test.go b/Router_test.go index 3f1c2fe..4d82073 100644 --- a/Router_test.go +++ b/Router_test.go @@ -4,9 +4,9 @@ import ( "strings" "testing" - "git.akyoto.dev/go/assert" - "git.akyoto.dev/go/router" - "git.akyoto.dev/go/router/testdata" + "git.urbach.dev/go/assert" + "git.urbach.dev/go/router" + "git.urbach.dev/go/router/testdata" ) func TestHello(t *testing.T) { diff --git a/benchmarks_test.go b/benchmarks_test.go index 55caf1f..41f3571 100644 --- a/benchmarks_test.go +++ b/benchmarks_test.go @@ -3,8 +3,8 @@ package router_test import ( "testing" - "git.akyoto.dev/go/router" - "git.akyoto.dev/go/router/testdata" + "git.urbach.dev/go/router" + "git.urbach.dev/go/router/testdata" ) func BenchmarkBlog(b *testing.B) { diff --git a/go.mod b/go.mod index 9f71fa8..b993518 100644 --- a/go.mod +++ b/go.mod @@ -1,5 +1,5 @@ -module git.akyoto.dev/go/router +module git.urbach.dev/go/router -go 1.22 +go 1.24 -require git.akyoto.dev/go/assert v0.1.3 +require git.urbach.dev/go/assert v0.0.0-20250225153414-fc1f84f19edf diff --git a/go.sum b/go.sum index 9fc2547..af0fe0b 100644 --- a/go.sum +++ b/go.sum @@ -1,2 +1,2 @@ -git.akyoto.dev/go/assert v0.1.3 h1:QwCUbmG4aZYsNk/OuRBz1zWVKmGlDUHhOnnDBfn8Qw8= -git.akyoto.dev/go/assert v0.1.3/go.mod h1:0GzMaM0eURuDwtGkJJkCsI7r2aUKr+5GmWNTFPgDocM= +git.urbach.dev/go/assert v0.0.0-20250225153414-fc1f84f19edf h1:BQWa5GKNUsA5CSUa/+UlFWYCEVe3IDDKRbVqBLK0mAE= +git.urbach.dev/go/assert v0.0.0-20250225153414-fc1f84f19edf/go.mod h1:y9jGII9JFiF1HNIju0u87OyPCt82xKCtqnAFyEreCDo= From ebcb5fad186c96dc2bfce40e550868e25cc889ec Mon Sep 17 00:00:00 2001 From: Eduard Urbach Date: Sat, 1 Mar 2025 14:27:26 +0100 Subject: [PATCH 09/10] Fixed incorrect path traversal --- README.md | 1 + Router_test.go | 30 ++++++++++++++++++++++++++++-- Tree.go | 17 +++++++++++++---- 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 6dd27c4..2532d80 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,7 @@ data := router.LookupNoAlloc("GET", "/users/42", func(key string, value string) ``` PASS: TestStatic PASS: TestParameter +PASS: TestMixed PASS: TestWildcard PASS: TestMap PASS: TestMethods diff --git a/Router_test.go b/Router_test.go index 4d82073..94ce016 100644 --- a/Router_test.go +++ b/Router_test.go @@ -72,9 +72,35 @@ func TestParameter(t *testing.T) { assert.Equal(t, data, "Comment") } +func TestMixed(t *testing.T) { + r := router.New[string]() + r.Add("GET", "/", "Frontpage") + r.Add("GET", "/blog", "Blog") + r.Add("GET", "/:post", "Post") + r.Add("GET", "/sitemap.txt", "Sitemap") + + data, params := r.Lookup("GET", "/") + assert.Equal(t, len(params), 0) + assert.Equal(t, data, "Frontpage") + + data, params = r.Lookup("GET", "/blog") + assert.Equal(t, len(params), 0) + assert.Equal(t, data, "Blog") + + data, params = r.Lookup("GET", "/software") + assert.Equal(t, len(params), 1) + assert.Equal(t, params[0].Key, "post") + assert.Equal(t, params[0].Value, "software") + assert.Equal(t, data, "Post") + + data, params = r.Lookup("GET", "/sitemap.txt") + assert.Equal(t, len(params), 0) + assert.Equal(t, data, "Sitemap") +} + func TestWildcard(t *testing.T) { r := router.New[string]() - r.Add("GET", "/", "Front page") + r.Add("GET", "/", "Frontpage") r.Add("GET", "/users/:id", "Parameter") r.Add("GET", "/images/static", "Static") r.Add("GET", "/images/*path", "Wildcard") @@ -84,7 +110,7 @@ func TestWildcard(t *testing.T) { data, params := r.Lookup("GET", "/") assert.Equal(t, len(params), 0) - assert.Equal(t, data, "Front page") + assert.Equal(t, data, "Frontpage") data, params = r.Lookup("GET", "/blog-post") assert.Equal(t, len(params), 1) diff --git a/Tree.go b/Tree.go index e1f6bb0..5a72491 100644 --- a/Tree.go +++ b/Tree.go @@ -92,10 +92,12 @@ func (tree *Tree[T]) Lookup(path string) (T, []Parameter) { // LookupNoAlloc finds the data for the given path without using any memory allocations. func (tree *Tree[T]) LookupNoAlloc(path string, addParameter func(key string, value string)) T { var ( - i uint - wildcardPath string - wildcard *treeNode[T] - node = &tree.root + i uint + parameterPath string + wildcardPath string + parameter *treeNode[T] + wildcard *treeNode[T] + node = &tree.root ) // Skip the first loop iteration if the starting characters are equal @@ -115,6 +117,8 @@ begin: wildcardPath = path[i:] } + parameter = node.parameter + parameterPath = path[i:] char := path[i] if char >= node.startIndex && char < node.endIndex { @@ -178,6 +182,11 @@ begin: // node: /|*any // path: /|image.png notFound: + if parameter != nil { + addParameter(parameter.prefix, parameterPath) + return parameter.data + } + if wildcard != nil { addParameter(wildcard.prefix, wildcardPath) return wildcard.data From e35d5715d1a5059fe07cfd1ff4c64515600f2197 Mon Sep 17 00:00:00 2001 From: Eduard Urbach Date: Sun, 1 Jun 2025 18:22:31 +0200 Subject: [PATCH 10/10] Updated dependencies --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index b993518..b147fdd 100644 --- a/go.mod +++ b/go.mod @@ -2,4 +2,4 @@ module git.urbach.dev/go/router go 1.24 -require git.urbach.dev/go/assert v0.0.0-20250225153414-fc1f84f19edf +require git.urbach.dev/go/assert v0.0.0-20250225153414-7a6ed8be9b6e diff --git a/go.sum b/go.sum index af0fe0b..049b850 100644 --- a/go.sum +++ b/go.sum @@ -1,2 +1,2 @@ -git.urbach.dev/go/assert v0.0.0-20250225153414-fc1f84f19edf h1:BQWa5GKNUsA5CSUa/+UlFWYCEVe3IDDKRbVqBLK0mAE= -git.urbach.dev/go/assert v0.0.0-20250225153414-fc1f84f19edf/go.mod h1:y9jGII9JFiF1HNIju0u87OyPCt82xKCtqnAFyEreCDo= +git.urbach.dev/go/assert v0.0.0-20250225153414-7a6ed8be9b6e h1:lDTetvmGktDiMem+iBU3e5cGv52qUIbsqW8sV9u3gAQ= +git.urbach.dev/go/assert v0.0.0-20250225153414-7a6ed8be9b6e/go.mod h1:y9jGII9JFiF1HNIju0u87OyPCt82xKCtqnAFyEreCDo=