Skip to content

Commit

Permalink
Merge 8362723 into 993a605
Browse files Browse the repository at this point in the history
  • Loading branch information
@sandeepboys
sandeepboys committed Mar 18, 2020
2 parents 993a605 + 8362723 commit 157c278
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 29 deletions.
37 changes: 22 additions & 15 deletions runtime/router/trie.go
View file
Expand Up @@ -114,7 +114,7 @@ func (t *Trie) Get(path string) (http.Handler, []Param, error) {
}
// ignore trailing slash
path = strings.TrimSuffix(path, "/")
return t.root.get(path, false, false, false)
return t.root.get(path, false, false, true)
}

// set sets the handler for given path, creates new child node if necessary
Expand All @@ -140,7 +140,7 @@ func (t *tnode) set(path string, value http.Handler, lastKeyCharSlash, lastPathC
// is immediately after slash, e.g. "/:foo", "/x/:y". "/a:b" is not a colon wildcard segment.
var keyMatchIdx, pathMatchIdx int
for keyMatchIdx < keyLength && pathMatchIdx < pathLength {
if (t.key[keyMatchIdx] == ':' && lastKeyCharSlash) ||
if (t.key[keyMatchIdx] == ':' && lastKeyCharSlash) &&
(path[pathMatchIdx] == ':' && lastPathCharSlash) {
keyStartIdx, pathStartIdx := keyMatchIdx, pathMatchIdx
same := t.key[keyMatchIdx] == path[pathMatchIdx]
Expand Down Expand Up @@ -171,7 +171,7 @@ func (t *tnode) set(path string, value http.Handler, lastKeyCharSlash, lastPathC
// already exists for the path.
if keyMatchIdx == keyLength {
for _, c := range t.children {
if _, _, err := c.get(path[pathMatchIdx:], lastKeyCharSlash, lastPathCharSlash, true); err == nil {
if _, _, err := c.get(path[pathMatchIdx:], lastKeyCharSlash, lastPathCharSlash, false); err == nil {
return errExist
}
}
Expand Down Expand Up @@ -205,7 +205,7 @@ func (t *tnode) set(path string, value http.Handler, lastKeyCharSlash, lastPathC
key: path[i:],
value: value,
}
t.children = append(t.children, newNode)
t.addChildren(newNode, lastPathCharSlash)
}
}

Expand Down Expand Up @@ -235,38 +235,50 @@ func (t *tnode) set(path string, value http.Handler, lastKeyCharSlash, lastPathC
key: path[i:],
value: value,
}
t.children = append(t.children, newNode)
t.addChildren(newNode, lastPathCharSlash)
}
}

return nil
}

func (t *tnode) addChildren(child *tnode, lastPathCharSlash bool) {
if lastPathCharSlash && child.key[0] != ':' {
// Prepending if child is not a pattern of :var
t.children = append([]*tnode{child}, t.children...)
} else {
// Appending if the child is of pattern :var
t.children = append(t.children, child)
}
}

func (t *tnode) get(path string, lastKeyCharSlash, lastPathCharSlash, colonAsPattern bool) (http.Handler, []Param, error) {
keyLength, pathLength := len(t.key), len(path)
var params []Param

// find the longest matched prefix
var keyIdx, pathIdx int
for keyIdx < keyLength && pathIdx < pathLength {
if t.key[keyIdx] == ':' && lastKeyCharSlash {
// wildcard starts - match until next slash
keyStartIdx, pathStartIdx := keyIdx+1, pathIdx
if t.key[keyIdx] == ':' && lastKeyCharSlash &&
path[pathIdx] == ':' && lastPathCharSlash {
keyStartIdx, pathStartIdx := keyIdx+1, pathIdx+1
for keyIdx < keyLength && t.key[keyIdx] != '/' {
keyIdx++
}
for pathIdx < pathLength && path[pathIdx] != '/' {
pathIdx++
}
params = append(params, Param{t.key[keyStartIdx:keyIdx], path[pathStartIdx:pathIdx]})
} else if path[pathIdx] == ':' && lastPathCharSlash && colonAsPattern {
// necessary for conflict check used in set call
} else if t.key[keyIdx] == ':' && lastKeyCharSlash && colonAsPattern {
// wildcard starts - match until next slash
keyStartIdx, pathStartIdx := keyIdx+1, pathIdx
for keyIdx < keyLength && t.key[keyIdx] != '/' {
keyIdx++
}
for pathIdx < pathLength && path[pathIdx] != '/' {
pathIdx++
}
params = append(params, Param{t.key[keyStartIdx:keyIdx], path[pathStartIdx:pathIdx]})
} else if t.key[keyIdx] == path[pathIdx] {
keyIdx++
pathIdx++
Expand All @@ -286,11 +298,6 @@ func (t *tnode) get(path string, lastKeyCharSlash, lastPathCharSlash, colonAsPat
return nil, nil, errNotFound
}

// ':' in path matches '*' in node key
if keyIdx > 0 && t.key[keyIdx-1] == '*' {
return t.value, params, nil
}

// longest matched prefix matches up to node key length and path length
if pathIdx == pathLength {
if t.value != nil {
Expand Down
51 changes: 37 additions & 14 deletions runtime/router/trie_test.go
View file
Expand Up @@ -135,10 +135,10 @@ func TestTriePathsWithPatten(t *testing.T) {
{op: set, path: "/a/:b", value: "baz", errMsg: errExist.Error()},
// test "/*" collides with "/a"
{op: set, path: "/*", value: "baz", errMsg: errExist.Error()},
// test "/:" collides with "/a"
{op: set, path: "/:x", value: "baz", errMsg: errExist.Error()},
// test "/:/b" collides with "/a/*"
{op: set, path: "/:x/b", value: "baz", errMsg: errExist.Error()},
// test "/:" should not collide with "/a"
{op: set, path: "/:x", value: "bar"},
// test "/:/b" should not collide with "/a/*"
{op: set, path: "/:x/b", value: "bar"},
}
runTrieTests(t, trie, tests)

Expand All @@ -159,7 +159,7 @@ func TestTriePathsWithPatten(t *testing.T) {
trie = NewTrie()
tests = []ts{
{op: set, path: "/:a", value: "foo"},
{op: get, path: "/:a", expectedValue: "foo", expectedParams: []Param{{"a", ":a"}}},
{op: get, path: "/:a", expectedValue: "foo", expectedParams: []Param{{"a", "a"}}},
}
runTrieTests(t, trie, tests)

Expand Down Expand Up @@ -187,19 +187,29 @@ func TestTriePathsWithPatten(t *testing.T) {

trie = NewTrie()
tests = []ts{
// test "/b" collides with "/:"
// test "/:a" does collide with "/:b"
{op: set, path: "/:a", value: "bar"},
{op: set, path: "/:b", value: "foo", errMsg: errExist.Error()},
}
runTrieTests(t, trie, tests)

trie = NewTrie()
tests = []ts{
// test "/b" does not collides with "/:"
{op: set, path: "/:a", value: "foo"},
{op: set, path: "/b", errMsg: errExist.Error()},
{op: set, path: "/b", value: "bar"},
{op: get, path: "/a/", expectedValue: "foo", expectedParams: []Param{{"a", "a"}}},
{op: get, path: "/b/", expectedValue: "bar"},
}
runTrieTests(t, trie, tests)

trie = NewTrie()
tests = []ts{
// test "/:" collides with "/b"
// test "/:" does not collides with "/b"
{op: set, path: "/b", value: "foo"},
{op: set, path: "/:a", errMsg: errExist.Error()},
{op: set, path: "/:a", value: "bar"},
{op: get, path: "/b/", expectedValue: "foo"},
{op: get, path: "/a/", expectedValue: "bar", expectedParams: []Param{{"a", "a"}}},
}
runTrieTests(t, trie, tests)

Expand All @@ -212,7 +222,7 @@ func TestTriePathsWithPatten(t *testing.T) {
{op: set, path: "/a/b/c/x", value: "2.1"},
{op: set, path: "/a/b/:cc/:d/e", value: "3"},
{op: set, path: "/a/b/c/d/f", value: "4"},
{op: set, path: "/a/:b/c/d", errMsg: errExist.Error()},
{op: set, path: "/a/:b/c/d", value: "5"},
{op: get, path: "/a/b/some/d", expectedValue: "2", expectedParams: []Param{{"cc", "some"}}},
{op: get, path: "/a/b/c/x", expectedValue: "2.1"},
{op: get, path: "/a/b/other/data/e", expectedValue: "3",
Expand All @@ -221,6 +231,7 @@ func TestTriePathsWithPatten(t *testing.T) {
{"d", "data"},
}},
{op: get, path: "/a/b/c/d/f", expectedValue: "4"},
{op: get, path: "/a/some/c/d", expectedValue: "5", expectedParams: []Param{{"b", "some"}}},
}
runTrieTests(t, trie, tests)

Expand All @@ -230,10 +241,18 @@ func TestTriePathsWithPatten(t *testing.T) {
{op: set, path: "/a/b", value: "1"},
{op: set, path: "/a/b/ccc/x", value: "2"},
{op: set, path: "/a/b/c/dope/f", value: "3"},
{op: set, path: "/a/b/ccc/:", errMsg: errExist.Error()},
{op: set, path: "/a/b/c/:/:/", errMsg: errExist.Error()},
{op: set, path: "/a/b/ccc/:", value: "4"},
{op: set, path: "/a/b/c/:/:/", value: "5"},
{op: get, path: "/a/b/ccc", errMsg: errNotFound.Error()},
{op: get, path: "/a/b/:", errMsg: errNotFound.Error()},
{op: get, path: "/a/b/ccc/x", expectedValue: "2"},
{op: get, path: "/a/b/ccc/y", expectedValue: "4", expectedParams: []Param{{"", "y"}}},
{op: get, path: "/a/b/c/dope/f", expectedValue: "3"},
{op: get, path: "/a/b/c/dope/g", expectedValue: "5",
expectedParams: []Param{
{"", "dope"},
{"", "g"},
}},
}
runTrieTests(t, trie, tests)

Expand All @@ -242,8 +261,12 @@ func TestTriePathsWithPatten(t *testing.T) {
// more ":" tests
{op: set, path: "/a/:b/c", value: "1"},
{op: set, path: "/a/:b/d", value: "2"},
{op: get, path: "/a/b/c", expectedValue: "1", expectedParams: []Param{{"b", "b"}}},
{op: get, path: "/a/b/d", expectedValue: "2", expectedParams: []Param{{"b", "b"}}},
{op: set, path: "/a/b/:c", value: "3"},
{op: set, path: "/a/b/c", value: "4"},
{op: get, path: "/a/x/c", expectedValue: "1", expectedParams: []Param{{"b", "x"}}},
{op: get, path: "/a/x/d", expectedValue: "2", expectedParams: []Param{{"b", "x"}}},
{op: get, path: "/a/b/d", expectedValue: "3", expectedParams: []Param{{"c", "d"}}},
{op: get, path: "/a/b/c", expectedValue: "4"},
}
runTrieTests(t, trie, tests)
}
Expand Down

0 comments on commit 157c278

Please sign in to comment.