Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support google custome verb #413

Merged
merged 6 commits into from Sep 11, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -68,3 +68,4 @@ examples/restful-html-template

s.html
restful-path-tail
.idea
13 changes: 11 additions & 2 deletions curly.go
Expand Up @@ -47,7 +47,7 @@ func (c CurlyRouter) SelectRoute(
func (c CurlyRouter) selectRoutes(ws *WebService, requestTokens []string) sortableCurlyRoutes {
candidates := make(sortableCurlyRoutes, 0, 8)
for _, each := range ws.routes {
matches, paramCount, staticCount := c.matchesRouteByPathTokens(each.pathParts, requestTokens)
matches, paramCount, staticCount := c.matchesRouteByPathTokens(each.pathParts, requestTokens, each.hasCustomVerb)
if matches {
candidates.add(curlyRoute{each, paramCount, staticCount}) // TODO make sure Routes() return pointers?
}
Expand All @@ -57,7 +57,7 @@ func (c CurlyRouter) selectRoutes(ws *WebService, requestTokens []string) sortab
}

// matchesRouteByPathTokens computes whether it matches, howmany parameters do match and what the number of static path elements are.
func (c CurlyRouter) matchesRouteByPathTokens(routeTokens, requestTokens []string) (matches bool, paramCount int, staticCount int) {
func (c CurlyRouter) matchesRouteByPathTokens(routeTokens, requestTokens []string, hasCustomVerb bool) (matches bool, paramCount int, staticCount int) {
if len(routeTokens) < len(requestTokens) {
// proceed in matching only if last routeToken is wildcard
count := len(routeTokens)
Expand All @@ -72,6 +72,15 @@ func (c CurlyRouter) matchesRouteByPathTokens(routeTokens, requestTokens []strin
return false, 0, 0
}
requestToken := requestTokens[i]
if hasCustomVerb{
if !isMatchCustomVerb(routeToken, requestToken) {
return false, 0, 0
}
staticCount++
requestToken = removeCustomVerb(requestToken)
routeToken = removeCustomVerb(routeToken)
}

if strings.HasPrefix(routeToken, "{") {
paramCount++
if colon := strings.Index(routeToken, ":"); colon != -1 {
Expand Down
31 changes: 19 additions & 12 deletions curly_test.go
Expand Up @@ -100,19 +100,26 @@ var routeMatchers = []struct {
matches bool
paramCount int
staticCount int
hasCustomVerb bool
}{
// route, request-path
{"/a", "/a", true, 0, 1},
{"/a", "/b", false, 0, 0},
{"/a", "/b", false, 0, 0},
{"/a/{b}/c/", "/a/2/c", true, 1, 2},
{"/{a}/{b}/{c}/", "/a/b", false, 0, 0},
{"/{x:*}", "/", false, 0, 0},
{"/{x:*}", "/a", true, 1, 0},
{"/{x:*}", "/a/b", true, 1, 0},
{"/a/{x:*}", "/a/b", true, 1, 1},
{"/a/{x:[A-Z][A-Z]}", "/a/ZX", true, 1, 1},
{"/basepath/{resource:*}", "/basepath/some/other/location/test.xml", true, 1, 1},
{"/a", "/a", true, 0, 1, false},
{"/a", "/b", false, 0, 0, false},
{"/a", "/b", false, 0, 0, false},
{"/a/{b}/c/", "/a/2/c", true, 1, 2, false},
{"/{a}/{b}/{c}/", "/a/b", false, 0, 0, false},
{"/{x:*}", "/", false, 0, 0, false},
{"/{x:*}", "/a", true, 1, 0, false},
{"/{x:*}", "/a/b", true, 1, 0, false},
{"/a/{x:*}", "/a/b", true, 1, 1, false},
{"/a/{x:[A-Z][A-Z]}", "/a/ZX", true, 1, 1, false},
{"/basepath/{resource:*}", "/basepath/some/other/location/test.xml", true, 1, 1, false},
{"/resources:run", "/resources:run", true, 0, 2, true},
{"/resources:run", "/user:run", false, 0, 0, true},
{"/resources:run", "/resources", false, 0, 0, true},
{"/{userId:^prefix-}:start", "/prefix-}:startUserId", false, 0, 0, true},
{"/{userId:^prefix-}:start", "/prefix-userId:start", true, 1, 1, true},

}

// clear && go test -v -test.run Test_matchesRouteByPathTokens ...restful
Expand All @@ -121,7 +128,7 @@ func Test_matchesRouteByPathTokens(t *testing.T) {
for i, each := range routeMatchers {
routeToks := tokenizePath(each.route)
reqToks := tokenizePath(each.path)
matches, pCount, sCount := router.matchesRouteByPathTokens(routeToks, reqToks)
matches, pCount, sCount := router.matchesRouteByPathTokens(routeToks, reqToks, each.hasCustomVerb)
if matches != each.matches {
t.Fatalf("[%d] unexpected matches outcome route:%s, path:%s, matches:%v", i, each.route, each.path, matches)
}
Expand Down
29 changes: 29 additions & 0 deletions custom_verb.go
@@ -0,0 +1,29 @@
package restful

import (
"fmt"
"regexp"
)

var (
customVerbReg = regexp.MustCompile(":([A-Za-z]+)$")
)

func hasCustomVerb(routeToken string) bool {
return customVerbReg.MatchString(routeToken)
}

func isMatchCustomVerb(routeToken string, pathToken string) bool {
rs := customVerbReg.FindStringSubmatch(routeToken)
if len(rs) < 2 {
return false
}

customVerb := rs[1]
specificVerbReg := regexp.MustCompile(fmt.Sprintf(":%s$", customVerb))
return specificVerbReg.MatchString(pathToken)
}

func removeCustomVerb(str string) string {
return customVerbReg.ReplaceAllString(str, "")
}
62 changes: 62 additions & 0 deletions custom_verb_test.go
@@ -0,0 +1,62 @@
package restful

import "testing"

func TestHasCustomVerb(t *testing.T) {
testCase := []struct {
path string
has bool
}{
{"/{userId}:init", true},
{"/{userId:init}", false},
{"/users/{id:init}:init", true},
{"/users/{id}", false},
}

for _, v := range testCase {
rs := hasCustomVerb(v.path)
if rs != v.has {
t.Errorf("path: %v should has no custom verb", v.path)
}
}
}

func TestRemoveCustomVerb(t *testing.T) {
testCase := []struct {
path string
expectedPath string
}{
{"/{userId}:init", "/{userId}"},
{"/{userId:init}", "/{userId:init}"},
{"/users/{id:init}:init", "/users/{id:init}"},
{"/users/{id}", "/users/{id}"},
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will fail: "/init/users/{id:init}:init" , "/init/users/{id:init}"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i added it to case, and run success, is something wrong?

	testCase := []struct{
		path string
		expectedPath string
	}{
		{"/{userId}:init", "/{userId}"},
		{"/{userId:init}", "/{userId:init}"},
		{"/users/{id:init}:init", "/users/{id:init}"},
		{"/users/{id}", "/users/{id}"},
		{"/init/users/{id:init}:init" , "/init/users/{id:init}"},
	}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tbh, i did not execute this test. i should have said "will this fail?". I came up with this test because I saw "ReplaceAllString" in removeCustomVerb. And this case "/:init/users:init/" is probably not a valid path, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

emmm, it likes "/users/{id:init}:init", so i think the case is not necessary.
and replace regex is ":customVerb$", it will just replace last word.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, good!

{"/init/users/{id:init}:init", "/init/users/{id:init}"},
}

for _, v := range testCase {
rs := removeCustomVerb(v.path)
if rs != v.expectedPath {
t.Errorf("expected value: %v, actual: %v", v.expectedPath, rs)
}
}
}
func TestMatchCustomVerb(t *testing.T) {
testCase := []struct {
routeToken string
pathToken string
expected bool
}{
{"{userId:regex}:init", "creator-123456789gWe:init", true},
{"{userId:regex}:init", "creator-123456789gWe", false},
{"{userId:regex}", "creator-123456789gWe:init", false},
{"users:init", "users:init", true},
{"users:init", "tokens:init", true},
}

for idx, v := range testCase {
rs := isMatchCustomVerb(v.routeToken, v.pathToken)
if rs != v.expected {
t.Errorf("expected value: %v, actual: %v, index: [%v]", v.expected, rs, idx)
}
}
}
3 changes: 3 additions & 0 deletions go.mod
@@ -0,0 +1,3 @@
module github.com/emicklei/go-restful

go 1.12
5 changes: 5 additions & 0 deletions path_processor.go
Expand Up @@ -29,6 +29,11 @@ func (d defaultPathProcessor) ExtractParameters(r *Route, _ *WebService, urlPath
} else {
value = urlParts[i]
}
if r.hasCustomVerb {
key = removeCustomVerb(key)
value = removeCustomVerb(value)
}

if strings.HasPrefix(key, "{") { // path-parameter
if colon := strings.Index(key, ":"); colon != -1 {
// extract by regex
Expand Down
25 changes: 25 additions & 0 deletions path_processor_test.go
Expand Up @@ -44,6 +44,31 @@ func TestExtractParameters_EmptyValue(t *testing.T) {
}
}

func TestExtractParameters_RegexAndCustomVerb(t *testing.T) {
testCase := []struct{
route string
size int
path string
checkList map[string]string
}{
{"/projects/{projectId}/users/{id:^prefix-}:custom", 4,"/projects/110/users/prefix-userId:custom", map[string]string{
"projectId": "110",
"id":"prefix-userId",}},
{"/projects/{projectId}/users/{id:*}", 4,"/projects/110/users/prefix-userId:custom", map[string]string{
"projectId": "110",
"id":"prefix-userId:custom",}},
}

for idx, v := range testCase {
params := doExtractParams(v.route, v.size, v.path, t)
for k, val := range v.checkList {
if params[k] != val{
t.Errorf("[%v] params: %v mismatch, expected: %v, actual: %v", idx, k, v.checkList[k], params[k])
}
}
}
}

func doExtractParams(routePath string, size int, urlPath string, t *testing.T) map[string]string {
r := Route{Path: routePath}
r.postBuild()
Expand Down
4 changes: 4 additions & 0 deletions route.go
Expand Up @@ -49,11 +49,15 @@ type Route struct {

//Overrides the container.contentEncodingEnabled
contentEncodingEnabled *bool

// indicate route path has custom verb
hasCustomVerb bool
}

// Initialize for Route
func (r *Route) postBuild() {
r.pathParts = tokenizePath(r.Path)
r.hasCustomVerb = hasCustomVerb(r.Path)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

}

// Create Request and Response from their http versions
Expand Down
10 changes: 10 additions & 0 deletions route_builder_test.go
Expand Up @@ -62,6 +62,16 @@ func TestRouteBuilder(t *testing.T) {
if r.DefaultResponse == nil {
t.Fatal("expected default response")
}
if r.hasCustomVerb {
t.Errorf("hasCustomVerb should not be true")
}

customVerbRoute := new(RouteBuilder)
customVerbRoute.To(dummy)
customVerbRoute.Path("/users:init")
if !customVerbRoute.Build().hasCustomVerb {
t.Errorf("hasCustomVerb should be true")
}
}

func TestAnonymousFuncNaming(t *testing.T) {
Expand Down