Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CONTEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,12 @@ segment. It is legal anywhere in a pattern, not only in the final position, so a
gap in its middle.
_Avoid_: a wildcard confined to one segment, a catch-all valid only as the last segment

**Whole-piece parameter**:
A `{name}` route segment names the whole piece between two slashes, never a part of one. A segment
mixing literal text with a parameter — `{id}c`, `x{id}`, `{a}-{b}` — is not a pattern govalin can
express, and a route carrying one fails at registration rather than matching nothing.
_Avoid_: a parameter embedded in a segment, a segment holding two parameters, a partial-piece capture

**Lifecycle breadth**:
Before and After handlers run for *every* route whose pattern matches the URL, while the request is
served by exactly one. Matching for the lifecycle is a collection; matching for the handler is a
Expand Down Expand Up @@ -455,3 +461,4 @@ _Avoid_: a before handler shadowed by an earlier match, lifecycle handlers resol
- `Vary` was read as naming the headers a response carries; resolved: it names the request headers the response was selected from, and a response header there is a cache key of nothing.
- Declaring a `Vary` was read as last-call-wins, the way a lifetime is; resolved: a lifetime is a policy one caller chooses for the response and a selecting header is a fact each layer knows a piece of, so declarations accumulate.
- "Who may store this" was read as something a cache key could answer; resolved: a key selects between stored responses, and a response that must not be shared at all is a cache scope decision.
- A route index was read as requiring most-specific-wins, the way the Go routers built on this shape resolve overlaps; resolved: **Registration-order match** is the rule an index has to preserve, so the tree carries the lowest registration order beneath each node and explores every branch that could beat the best it has.
70 changes: 70 additions & 0 deletions docs/adr/0015-segment-tree-route-index.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# A segment tree that keeps registration order

## Status

accepted

## Context and decision

Matching a URL asked every registered route whether it matched. Each route held a compiled regexp,
so a 200 route table ran 200 regexp executions per request: 63% of the request's time, plus the
`sync.Pool` traffic of taking a regexp machine out and putting it back 200 times. What a route cost
depended on where its author had happened to register it — 331ns at the top of the table, 4611ns at
the bottom.

Routes are now indexed by their segments. A literal segment is a key, a parameter segment is the one
child that takes any piece, and a route sits at the node its last segment reaches; the walk descends
the URL's pieces rather than the table. A match costs what the path is, not what the table holds.

### Registration order is the constraint, not a detail to trade away

Every other Go router of this shape resolves overlapping routes by specificity: a static segment
beats a parameter, a parameter beats a wildcard. Govalin's **Registration-order match** says the
opposite — the first route registered whose pattern matches is the one that serves, so a literal
registered after a parameter route that also matches is unreachable *by design*. That rule is the
author's tool for deciding which of two overlapping patterns wins, and an index that quietly replaced
it with specificity would change which handler answers an existing app's requests.

So the tree preserves it. Each node carries the lowest registration order of any route beneath it,
and the walk carries the best order it has found; a subtree that cannot beat it is never entered.
Parameter and literal children are both explored, because either may hold the earlier route.

### The tree indexes segments, so what is not a segment stays in a list

A wildcard spans separators, which is not a shape a per-segment tree can key on. Rather than
complicate the tree with a construct that appears a handful of times in an app — a static mount, a
CORS `Before`, an HTTPS redirect — routes holding a wildcard stay in an ordered list scanned after
the walk, where the lowest order still wins. An app that registers many wildcard routes gets the
linear scan it always had, now over a segment matcher rather than a regexp.

The same reasoning removed the regexps: the patterns the parser built were only ever literals,
`([^/]+?)` and `.+?` joined by slashes, which is a walk over slash-separated pieces expressed in a
language that cannot know that. `PathMatcher` walks its segments directly and captures parameters on
the same pass instead of running the pattern a second time for submatches.

## Considered options

- **A segment tree ordered by registration (chosen)** — see above.
- **A specificity tree, as httprouter and gin use** — simpler, no order bookkeeping, and faster still
because only one branch is ever explored. Rejected because it silently changes which handler serves
a URL in any app with overlapping routes.
- **Keeping the linear scan and only replacing the regexp** — this was measured: it took the bottom
of a 200 route table from 4350ns to 1950ns for a fraction of the code. Rejected as the endpoint
rather than the step it became, because the cost still scales with the table.
- **Making the tree the whole answer and confirming nothing** — the segments settle everything about
a match except how the path spells its end. Rather than re-run the full match to check, the matcher
reports whether its trailing slash is optional and the walk asks at the node.
- **A slice of literal children instead of a map** — measured 5ns faster per request on small tables
and 40ns slower on a node with 40 children. Rejected: the map is the one that does not reintroduce
a scan proportional to the route table, which is the thing being fixed.

## Consequences

- A route table's shape no longer changes what a request costs: 120ns at the top of a 200 route
table and 271ns at the bottom, against 331ns and 4611ns before, and a one route app is unchanged.
- Every route now exists twice, in `pathHandlers` and in the index. `getOrCreatePathHandlerByPath` is
the single place a route is created, so it is the single place the two can diverge.
- The index is only correct if it agrees with a scan of the table. That is a property rather than a
case list, so it is tested as one, against 20k generated route tables.
- Before and after handlers still scan, over lists holding only the routes that have one. They match
every route they hit rather than the first, so the index's answer is not the question they ask.
7 changes: 4 additions & 3 deletions handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
)

type pathHandler struct {
order int
PathFragment string
PathMatcher routing.PathMatcher
Before BeforeFunc
Expand All @@ -21,15 +22,15 @@ type pathHandler struct {
Options HandlerFunc
}

func newPathHandlerFromPathFragment(pathFragment string) (pathHandler, error) {
func newPathHandlerFromPathFragment(pathFragment string) (*pathHandler, error) {
pathMatcher, err := routing.NewPathMatcherFromString(pathFragment)
if err != nil {
return pathHandler{}, fmt.Errorf(
return nil, fmt.Errorf(
"failed to create path matcher for pathFragment '%s'. Err: %w", pathFragment, err,
)
}

return pathHandler{
return &pathHandler{
PathFragment: pathFragment,
PathMatcher: pathMatcher,
Head: nil,
Expand Down
172 changes: 130 additions & 42 deletions internal/routing/path-parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,19 @@ package routing
import (
"fmt"
"log/slog"
"regexp"
"strings"
)

// PathMatcher matches request URLs against one compiled route path.
type PathMatcher struct {
pathParamNames []string
regexp *regexp.Regexp
segments []Segment
pathParamNames []string
optionalTrailingSlash bool
matchesEverything bool
}

// NewPathMatcherFromString compiles a route path into a matcher. It fails on a
// path segment that is neither a literal, a '{name}' parameter nor a wildcard.
func NewPathMatcherFromString(path string) (PathMatcher, error) {
// A path of nothing but slashes is the root spelled oddly, and a route fragment
// joined to a path produces it. The matcher has to match what the warning says it became.
Expand All @@ -20,17 +24,11 @@ func NewPathMatcherFromString(path string) (PathMatcher, error) {
slog.Warn(fmt.Sprintf("The path '%s' was converted to /", path))
}

return PathMatcher{
pathParamNames: []string{},
regexp: regexp.MustCompile("^/$"),
}, nil
return PathMatcher{}, nil
}

if path == "*" {
return PathMatcher{
pathParamNames: []string{},
regexp: regexp.MustCompile(".*?"),
}, nil
if path == wildcard {
return PathMatcher{matchesEverything: true}, nil
}

pathSegments, err := getPathSegments(path)
Expand All @@ -39,33 +37,21 @@ func NewPathMatcherFromString(path string) (PathMatcher, error) {
}

pathParamNames := []string{}
for _, ps := range pathSegments {
pathParamNames = append(pathParamNames, ps.PathNames...)
}

groupRegexpParts := []string{}

for _, ps := range pathSegments {
groupRegexpParts = append(groupRegexpParts, ps.GroupedRegex)
}

// Appended after the join, not as a segment: a segment carries a mandatory slash in
// front of the optional one, leaving the pattern matching only the doubled form.
optionalTrailingSlash := ""
if strings.HasSuffix(path, "/") {
optionalTrailingSlash = "/?"
for _, segment := range pathSegments {
if segment.Kind == ParameterSegment {
pathParamNames = append(pathParamNames, segment.Text)
}
}

fullGroupedRegexpString := "^/" + strings.Join(groupRegexpParts, "/") + optionalTrailingSlash + "$"

return PathMatcher{
pathParamNames: pathParamNames,
regexp: regexp.MustCompile(fullGroupedRegexpString),
segments: pathSegments,
pathParamNames: pathParamNames,
optionalTrailingSlash: strings.HasSuffix(path, "/"),
}, nil
}

func getPathSegments(path string) ([]pathSegment, error) {
pathSegments := []pathSegment{}
func getPathSegments(path string) ([]Segment, error) {
pathSegments := []Segment{}
pathParts := strings.Split(path, "/")

for _, pathPiece := range pathParts {
Expand All @@ -86,7 +72,9 @@ func getPathSegments(path string) ([]pathSegment, error) {

// MatchesURL checks whether given string URL matches the path.
func (path *PathMatcher) MatchesURL(url string) bool {
return path.regexp.MatchString(url)
_, matches := path.match(url, nil)

return matches
}

// PathParams extracts the path parameters from given string url according
Expand All @@ -100,17 +88,117 @@ func (path *PathMatcher) PathParams(url string) map[string]string {
return nil
}

pathparamMap := map[string]string{}
pathParams := path.regexp.FindStringSubmatch(url)
values, matches := path.match(url, make([]string, 0, len(path.pathParamNames)))
if !matches {
slog.Error(fmt.Sprintf("The URL '%s' does not match the path it was asked for path params on", url))
return map[string]string{}
}

pathParamMap := make(map[string]string, len(values))
for i, name := range path.pathParamNames {
pathParamMap[name] = values[i]
}

return pathParamMap
}

// Segments returns the pieces the path compiled to, and whether the path is
// nothing but those pieces. A caller that indexes routes by their segments can
// key on the ones it gets; a path holding a wildcard, or one that matches every
// URL, matches by a rule its segments do not carry and reports false.
func (path *PathMatcher) Segments() ([]Segment, bool) {
if path.matchesEverything {
return nil, false
}

for _, segment := range path.segments {
if segment.Kind == WildcardSegment {
return nil, false
}
}

return path.segments, true
}

// OptionalTrailingSlash reports whether the path was registered with a trailing
// slash, and so matches a URL spelled either way. A caller that decided a match
// by the segments alone still has to ask: the slash is a rule about the end of
// the path rather than a piece of it.
func (path *PathMatcher) OptionalTrailingSlash() bool {
return path.optionalTrailingSlash
}

// match walks the URL across the compiled segments, appending the values it
// captures to values. A nil values skips capture, so the routes a request does
// not match cost nothing to rule out.
func (path *PathMatcher) match(url string, values []string) ([]string, bool) {
if path.matchesEverything {
return values, true
}

if len(url) == 0 || url[0] != '/' {
return values, false
}

return path.matchSegments(path.segments, url[1:], values)
}

// matchSegments matches segments against a URL positioned at the first
// character of the first segment.
func (path *PathMatcher) matchSegments(segments []Segment, url string, values []string) ([]string, bool) {
if len(segments) == 0 {
return values, url == "" || (path.optionalTrailingSlash && url == "/")
}

head, rest := segments[0], segments[1:]

switch head.Kind {
case LiteralSegment:
if !strings.HasPrefix(url, head.Text) {
return values, false
}

return path.matchSeparator(rest, url[len(head.Text):], values)

case ParameterSegment:
end := strings.IndexByte(url, '/')
if end < 0 {
end = len(url)
}
if end == 0 {
return values, false
}
if values != nil {
values = append(values, url[:end])
}

return path.matchSeparator(rest, url[end:], values)
}

// A wildcard spans separators and takes as little as it can, so the shortest
// remainder that lets the segments after it match is the one that wins.
if len(rest) == 0 {
return values, url != ""
}

for consumed := 1; consumed < len(url); consumed++ {
if captured, matches := path.matchSeparator(rest, url[consumed:], values); matches {
return captured, true
}
}

return values, false
}

if len(pathParams) != len(path.pathParamNames)+1 {
slog.Error("The number of path params is not the same as configured path names")
return pathparamMap
// matchSeparator consumes the slash between two segments.
func (path *PathMatcher) matchSeparator(segments []Segment, url string, values []string) ([]string, bool) {
if len(segments) == 0 {
return path.matchSegments(segments, url, values)
}

for i, v := range path.pathParamNames {
pathparamMap[v] = pathParams[i+1]
if len(url) == 0 || url[0] != '/' {
return values, false
}

return pathparamMap
return path.matchSegments(segments, url[1:], values)
}
53 changes: 53 additions & 0 deletions internal/routing/path-parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,56 @@ func TestNestedWildcardMatch(t *testing.T) {
assert.Equal(t, false, pathMatcher.MatchesURL("/baz/baz/foo"), "Should not match on mismatched wildcard")
assert.Equal(t, false, pathMatcher.MatchesURL("/foo/baz"), "Should not match on mismatched wildcard")
}

// A segment mixing literal text and a parameter used to silently compile to a
// pattern matching nothing its author meant; now it fails at registration.
func TestMalformedParameterSegmentIsRejected(t *testing.T) {
for _, path := range []string{"/a/{b}c", "/a/x{b}", "/a/{b}-{c}", "/a/{}", "/a/{{b}}", "/a/{b", "/a/b}"} {
_, err := routing.NewPathMatcherFromString(path)

assert.Error(t, err, "'%s' is not a path the matcher can express", path)
}
}

// TestParameterSegmentSpansTheWholePiece keeps the shapes either side of the
// rejection above matching what they always did.
func TestParameterSegmentSpansTheWholePiece(t *testing.T) {
pathMatcher, err := routing.NewPathMatcherFromString("/a/{b}")

assert.Nil(t, err)
assert.Equal(t, true, pathMatcher.MatchesURL("/a/foo"), "Should match a value in the parameter position")
assert.Equal(t, map[string]string{"b": "foo"}, pathMatcher.PathParams("/a/foo"), "Should capture the value")
assert.Equal(t, false, pathMatcher.MatchesURL("/a/"), "Should not match an empty parameter")
assert.Equal(t, false, pathMatcher.MatchesURL("/a/foo/bar"), "Should not span a separator")
}

// TestWildcardBacktracksToTheSegmentsAfterIt covers the case a segment walk has
// to get right and a left-to-right scan does not: a wildcard takes as little as
// it can, so how much of the URL it swallows is only settled by whether the
// segments after it still match.
func TestWildcardBacktracksToTheSegmentsAfterIt(t *testing.T) {
pathMatcher, err := routing.NewPathMatcherFromString("/a/*/b/{id}")
assert.Nil(t, err)

assert.Equal(t, true, pathMatcher.MatchesURL("/a/x/b/1"), "Should match with the wildcard taking one piece")
assert.Equal(t, true, pathMatcher.MatchesURL("/a/x/y/z/b/1"), "Should match with the wildcard spanning separators")
assert.Equal(t, true, pathMatcher.MatchesURL("/a/b/b/1"), "Should pass over a piece the later literal also matches")
assert.Equal(t, false, pathMatcher.MatchesURL("/a/b/1"), "Should not match with nothing left for the wildcard")
assert.Equal(t, false, pathMatcher.MatchesURL("/a/x/b/1/2"), "Should not let the parameter span a separator")

assert.Equal(
t,
map[string]string{"id": "1"},
pathMatcher.PathParams("/a/x/y/b/1"),
"Should capture from the match the wildcard backtracked into",
)
}

// TestWildcardMatchesAnyCharacter pins the wildcard as any character: the
// regexp it replaced skipped a newline a parameter in the same position accepted.
func TestWildcardMatchesAnyCharacter(t *testing.T) {
pathMatcher, err := routing.NewPathMatcherFromString("/a/*")

assert.Nil(t, err)
assert.Equal(t, true, pathMatcher.MatchesURL("/a/x\ny"), "Should match a path holding a newline")
}
Loading
Loading