Skip to content

Commit

Permalink
fixes Issue #137 , added tests for both routers, updated coverage script
Browse files Browse the repository at this point in the history
  • Loading branch information
emicklei committed Aug 23, 2014
1 parent 946c331 commit 35a224b
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 25 deletions.
5 changes: 1 addition & 4 deletions container.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,6 @@ func (c *Container) EnableContentEncoding(enabled bool) {

// Add a WebService to the Container. It will detect duplicate root paths and panic in that case.
func (c *Container) Add(service *WebService) *Container {
if service.pathExpr == nil {
service.Path("/") // lazy initialize path
}
// If registered on root then no additional specific mapping is needed
if !c.isRegisteredOnRoot {
pattern := c.fixedPrefixPath(service.RootPath())
Expand Down Expand Up @@ -233,7 +230,7 @@ func (c Container) computeAllowedMethods(req *Request) []string {
methods := []string{}
requestPath := req.Request.URL.Path
for _, ws := range c.RegisteredWebServices() {
matches := ws.pathExpr.Matcher.FindStringSubmatch(requestPath)
matches := ws.compiledPathExpression().Matcher.FindStringSubmatch(requestPath)
if matches != nil {
finalMatch := matches[len(matches)-1]
for _, rt := range ws.Routes() {
Expand Down
7 changes: 2 additions & 5 deletions coverage.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,2 @@
# https://github.com/matm/gocov-html
# go get github.com/axw/gocov/gocov
# go get github.com/matm/gocov-html

gocov test github.com/emicklei/go-restful | gocov-html > restful.html && open restful.html
go test -coverprofile=coverage.out
go tool cover -html=coverage.out
2 changes: 1 addition & 1 deletion curly.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (c CurlyRouter) detectWebService(requestTokens []string, webServices []*Web
var best *WebService
score := -1
for _, each := range webServices {
matches, eachScore := c.computeWebserviceScore(requestTokens, each.pathExpr.tokens)
matches, eachScore := c.computeWebserviceScore(requestTokens, each.compiledPathExpression().tokens)
if matches && (eachScore > score) {
best = each
score = eachScore
Expand Down
20 changes: 16 additions & 4 deletions curly_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestCurlyDetectWebService(t *testing.T) {
var wss = []*WebService{ws1, ws2, ws3, ws4, ws5, ws7}

for _, each := range wss {
t.Logf("path=%s,toks=%v\n", each.pathExpr.Source, each.pathExpr.tokens)
t.Logf("path=%s,toks=%v\n", each.compiledPathExpression().Source, each.compiledPathExpression().tokens)
}

router := CurlyRouter{}
Expand Down Expand Up @@ -79,7 +79,7 @@ func Test_detectWebService(t *testing.T) {
requestPath := fix.path
requestTokens := tokenizePath(requestPath)
for _, ws := range wss {
serviceTokens := ws.pathExpr.tokens
serviceTokens := ws.compiledPathExpression().tokens
matches, score := router.computeWebserviceScore(requestTokens, serviceTokens)
t.Logf("req=%s,toks:%v,ws=%s,toks:%v,score=%d,matches=%v", requestPath, requestTokens, ws.RootPath(), serviceTokens, score, matches)
}
Expand Down Expand Up @@ -166,7 +166,7 @@ func TestCurly_ISSUE_34(t *testing.T) {

// clear && go test -v -test.run TestCurly_ISSUE_34_2 ...restful
func TestCurly_ISSUE_34_2(t *testing.T) {
ws1 := new(WebService).Path("/")
ws1 := new(WebService)
ws1.Route(ws1.GET("/network/{id}").To(curlyDummy))
ws1.Route(ws1.GET("/{type}/{id}").To(curlyDummy))
routes := CurlyRouter{}.selectRoutes(ws1, tokenizePath("/network/12"))
Expand All @@ -180,7 +180,7 @@ func TestCurly_ISSUE_34_2(t *testing.T) {

// clear && go test -v -test.run TestCurly_JsonHtml ...restful
func TestCurly_JsonHtml(t *testing.T) {
ws1 := new(WebService).Path("/")
ws1 := new(WebService)
ws1.Route(ws1.GET("/some.html").To(curlyDummy).Consumes("*/*").Produces("text/html"))
req, _ := http.NewRequest("GET", "/some.html", nil)
req.Header.Set("Accept", "application/json")
Expand All @@ -193,4 +193,16 @@ func TestCurly_JsonHtml(t *testing.T) {
}
}

// go test -v -test.run TestCurly_ISSUE_137 ...restful
func TestCurly_ISSUE_137(t *testing.T) {
ws1 := new(WebService)
ws1.Route(ws1.GET("/hello").To(curlyDummy))
req, _ := http.NewRequest("GET", "/", nil)
_, route, _ := CurlyRouter{}.SelectRoute([]*WebService{ws1}, req)
t.Log(route)
if route != nil {
t.Error("no route expected")
}
}

func curlyDummy(req *Request, resp *Response) { io.WriteString(resp.ResponseWriter, "curlyDummy") }
9 changes: 3 additions & 6 deletions jsr311.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,16 +92,13 @@ func (r RouterJSR311) bestMatchByMedia(routes []Route, contentType string, accep

// http://jsr311.java.net/nonav/releases/1.1/spec/spec3.html#x3-360003.7.2 (step 2)
func (r RouterJSR311) selectRoutes(dispatcher *WebService, pathRemainder string) []Route {
if pathRemainder == "" || pathRemainder == "/" {
return dispatcher.Routes()
}
filtered := &sortableRouteCandidates{}
for _, each := range dispatcher.Routes() {
pathExpr := each.pathExpr
matches := pathExpr.Matcher.FindStringSubmatch(pathRemainder)
if matches != nil {
lastMatch := matches[len(matches)-1]
if lastMatch == "" || lastMatch == "/" { // do not include if value is neither empty nor ‘/’.
if len(lastMatch) == 0 || lastMatch == "/" { // do not include if value is neither empty nor ‘/’.
filtered.candidates = append(filtered.candidates,
routeCandidate{each, len(matches) - 1, pathExpr.LiteralCount, pathExpr.VarCount})
}
Expand All @@ -123,11 +120,11 @@ func (r RouterJSR311) selectRoutes(dispatcher *WebService, pathRemainder string)
return matchingRoutes
}

// http://jsr311.java.net/nonav/releases/1.1/spec/spec3.html#x3-360003.7.2
// http://jsr311.java.net/nonav/releases/1.1/spec/spec3.html#x3-360003.7.2 (step 1)
func (r RouterJSR311) detectDispatcher(requestPath string, dispatchers []*WebService) (*WebService, string, error) {
filtered := &sortableDispatcherCandidates{}
for _, each := range dispatchers {
pathExpr := each.pathExpr
pathExpr := each.compiledPathExpression()
matches := pathExpr.Matcher.FindStringSubmatch(requestPath)
if matches != nil {
filtered.candidates = append(filtered.candidates,
Expand Down
22 changes: 22 additions & 0 deletions jsr311_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,17 @@ func TestISSUE_34_2(t *testing.T) {
}
}

// go test -v -test.run TestISSUE_137 ...restful
func TestISSUE_137(t *testing.T) {
ws1 := new(WebService)
ws1.Route(ws1.GET("/hello").To(dummy))
routes := RouterJSR311{}.selectRoutes(ws1, "/")
t.Log(routes)
if len(routes) > 0 {
t.Error("no route expected")
}
}

func TestSelectRoutesSlash(t *testing.T) {
ws1 := new(WebService).Path("/")
ws1.Route(ws1.GET("").To(dummy))
Expand All @@ -117,6 +128,9 @@ func TestSelectRoutesSlash(t *testing.T) {
ws1.Route(ws1.POST("/u/{w}/z").To(dummy))
routes := RouterJSR311{}.selectRoutes(ws1, "/u")
checkRoutesContains(routes, "/u", t)
checkRoutesContainsNo(routes, "/u/v", t)
checkRoutesContainsNo(routes, "/", t)
checkRoutesContainsNo(routes, "/u/{w}/z", t)
}
func TestSelectRoutesU(t *testing.T) {
ws1 := new(WebService).Path("/u")
Expand Down Expand Up @@ -145,6 +159,14 @@ func checkRoutesContains(routes []Route, path string, t *testing.T) {
t.Fatalf("routes should include [%v]:", path)
}
}
func checkRoutesContainsNo(routes []Route, path string, t *testing.T) {
if containsRoutePath(routes, path, t) {
for _, r := range routes {
t.Logf("route %v %v", r.Method, r.Path)
}
t.Fatalf("routes should not include [%v]:", path)
}
}
func containsRoutePath(routes []Route, path string, t *testing.T) bool {
for _, each := range routes {
if each.Path == path {
Expand Down
20 changes: 15 additions & 5 deletions web_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,25 @@ type WebService struct {
documentation string
}

// compiledPathExpression ensures that the path is compiled into a RegEx for those routers that need it.
func (w *WebService) compiledPathExpression() *pathExpression {
if w.pathExpr == nil {
if len(w.rootPath) == 0 {
w.Path("/") // lazy initialize path
}
compiled, err := newPathExpression(w.rootPath)
if err != nil {
log.Fatalf("[restful] Invalid path:%s because:%v", w.rootPath, err)
}
w.pathExpr = compiled
}
return w.pathExpr
}

// Path specifies the root URL template path of the WebService.
// All Routes will be relative to this path.
func (w *WebService) Path(root string) *WebService {
w.rootPath = root
compiled, err := newPathExpression(root)
if err != nil {
log.Fatalf("[restful] Invalid path:%s because:%v", root, err)
}
w.pathExpr = compiled
return w
}

Expand Down

0 comments on commit 35a224b

Please sign in to comment.