Skip to content

Commit

Permalink
v3: Fix trailing slash problems (goadesign#2560)
Browse files Browse the repository at this point in the history
* Add trailing slash tests

* Fix trailing slash

* Fix a comment

* Add description to Path DSL

* Fix typo
  • Loading branch information
ikawaha committed May 21, 2020
1 parent 280957c commit 191d509
Show file tree
Hide file tree
Showing 7 changed files with 146 additions and 27 deletions.
5 changes: 5 additions & 0 deletions dsl/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,11 @@ func Produces(args ...string) {
// description of the wildcard syntax). The corresponding parameters must be
// described using Params. Multiple base paths may be defined for services.
//
// GET("/") does not add a trailing slash when the base path is defined by Path.
// For example, when Path('foo') is defined, the path generated by GET("/") will be '/foo'.
// As a special case, if you want to generate a path with a trailing slash, you can use
// GET("/./") to generate a path such as '/foo/'.
//
// Path must appear in a API HTTP expression or a Service HTTP expression.
//
// Path accepts one argument: the HTTP path prefix.
Expand Down
8 changes: 7 additions & 1 deletion expr/http_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,13 @@ func (r *RouteExpr) FullPaths() []string {
res := make([]string, len(bases))
for i, b := range bases {
res[i] = httppath.Clean(path.Join(b, r.Path))
if res[i] != "/" && strings.HasSuffix(r.Path, "/") {
if res[i] == "/" {
continue
}
// path has trailing slash
if r.Path == "/" && strings.HasSuffix(b, "/") {
res[i] += "/"
} else if r.Path != "/" && strings.HasSuffix(r.Path, "/") {
res[i] += "/"
}
}
Expand Down
7 changes: 6 additions & 1 deletion expr/http_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,12 @@ func (svc *HTTPServiceExpr) FullPaths() []string {
basePaths = []string{Root.API.HTTP.Path}
}
for _, base := range basePaths {
paths = append(paths, httppath.Clean(path.Join(base, p)))
v := httppath.Clean(path.Join(base, p))
// path has trailing slash
if strings.HasSuffix(p, "/") {
v += "/"
}
paths = append(paths, v)
}
}
return paths
Expand Down
32 changes: 30 additions & 2 deletions http/codegen/paths_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ func TestPaths(t *testing.T) {
Code string
}{
{"single-path-no-param", testdata.PathNoParamDSL, testdata.PathNoParamCode},
{"single-path-no-param-trailing-slash", testdata.PathNoParamTrailingSlashDSL, testdata.PathNoParamTrailingSlashCode},
{"single-path-one-param", testdata.PathOneParamDSL, testdata.PathOneParamCode},
{"single-path-one-param-trailing-slash", testdata.PathOneParamTrailingSlashDSL, testdata.PathOneParamTrailingSlashCode},
{"single-path-multiple-params", testdata.PathMultipleParamsDSL, testdata.PathMultipleParamsCode},
{"alternative-paths", testdata.PathAlternativesDSL, testdata.PathAlternativesCode},
{"path-with-string-slice-param", testdata.PathStringSliceParamDSL, testdata.PathStringSliceParamCode},
Expand Down Expand Up @@ -48,3 +46,33 @@ func TestPaths(t *testing.T) {
})
}
}

func TestPathTrailingShash(t *testing.T) {
cases := []struct {
Name string
DSL func()
Code string
}{
{"slash_with_base_path_no_trailing", testdata.BasePathNoTrailing_SlashWithBasePathNoTrailingDSL, testdata.BasePathNoTrailing_SlashWithBasePathNoTrailingCode},
{"trailing_with_base_path_no_trailing", testdata.BasePathNoTrailing_TrailingWithBasePathNoTrailingDSL, testdata.BasePathNoTrailing_TrailingWithBasePathNoTrailingCode},
{"slash_with_base_path_with_trailing", testdata.BasePathWithTrailingSlash_WithBasePathWithTrailingDSL, testdata.BasePathWithTrailingSlash_WithBasePathWithTrailingCode},
{"slash_no_base_path", testdata.NoBasePath_SlashNoBasePathDSL, testdata.NoBasePath_SlashNoBasePathCode},
{"path-trailing_no_base_path", testdata.NoBasePath_TrailingNoBasePathDSL, testdata.NoBasePath_TrailingNoBasePathCode},
{"add-trailing-slash-to-base-path", testdata.BasePath_SpecialTrailingSlashDSL, testdata.BasePath_SpecialTrailingSlashCode},
}

for _, c := range cases {
t.Run(c.Name, func(t *testing.T) {
RunHTTPDSL(t, c.DSL)
if len(expr.Root.API.HTTP.Services) != 1 {
t.Fatalf("got %d file(s), expected 1", len(expr.Root.API.HTTP.Services))
}
fs := serverPath(expr.Root.API.HTTP.Services[0])
sections := fs.SectionTemplates
code := codegen.SectionCode(t, sections[1])
if code != c.Code {
t.Errorf("invalid code, got:\n%s\ngot vs. expected:\n%s", code, codegen.Diff(t, code, c.Code))
}
})
}
}
77 changes: 64 additions & 13 deletions http/codegen/testdata/path_dsls.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,33 +14,84 @@ var PathNoParamDSL = func() {
})
}

var PathNoParamTrailingSlashDSL = func() {
Service("ServicePathNoParamTrailingSlash", func() {
Method("MethodPathNoParamTrailingSlash", func() {
var BasePathNoTrailing_SlashWithBasePathNoTrailingDSL = func() {
Service("BasePathNoTrailing", func() {
HTTP(func() {
Path("foo")
})
Method("SlashWithBasePathNoTrailing", func() {
HTTP(func() {
GET("/one/two/")
GET("/")
})
})
})
}

var PathOneParamDSL = func() {
Service("ServicePathOneParam", func() {
Method("MethodPathOneParam", func() {
Payload(String)
var BasePathNoTrailing_TrailingWithBasePathNoTrailingDSL = func() {
Service("BasePathNoTrailing", func() {
HTTP(func() {
Path("foo")
})
Method("TrailingWithBasePathNoTrailing", func() {
HTTP(func() {
GET("one/{a}/two")
GET("/bar/")
})
})
})
}

var PathOneParamTrailingSlashDSL = func() {
Service("ServicePathOneParamTrailingSlash", func() {
Method("MethodPathOneParamTrailingSlash", func() {
var BasePathWithTrailingSlash_WithBasePathWithTrailingDSL = func() {
Service("BasePathWithTrailing", func() {
HTTP(func() {
Path("foo/")
})
Method("SlashWithBasePathWithTrailing", func() {
HTTP(func() {
GET("/")
})
})
})
}

var NoBasePath_SlashNoBasePathDSL = func() {
Service("NoBasePath", func() {
Method("SlashNoBasePath", func() {
HTTP(func() {
GET("/")
})
})
})
}

var NoBasePath_TrailingNoBasePathDSL = func() {
Service("NoBasePath", func() {
Method("TrailingNoBasePath", func() {
HTTP(func() {
GET("/foo/")
})
})
})
}

var BasePath_SpecialTrailingSlashDSL = func() {
Service("BasePath", func() {
HTTP(func() {
Path("/foo")
})
Method("SpecialTrailingSlash", func() {
HTTP(func() {
GET("/./")
})
})
})
}

var PathOneParamDSL = func() {
Service("ServicePathOneParam", func() {
Method("MethodPathOneParam", func() {
Payload(String)
HTTP(func() {
GET("one/{a}/")
GET("one/{a}/two")
})
})
})
Expand Down
42 changes: 33 additions & 9 deletions http/codegen/testdata/path_functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,45 @@ func MethodPathNoParamServicePathNoParamPath() string {
}
`

var PathNoParamTrailingSlashCode = `// MethodPathNoParamTrailingSlashServicePathNoParamTrailingSlashPath returns the URL path to the ServicePathNoParamTrailingSlash service MethodPathNoParamTrailingSlash HTTP endpoint.
func MethodPathNoParamTrailingSlashServicePathNoParamTrailingSlashPath() string {
return "/one/two/"
var BasePathNoTrailing_SlashWithBasePathNoTrailingCode = `// SlashWithBasePathNoTrailingBasePathNoTrailingPath returns the URL path to the BasePathNoTrailing service SlashWithBasePathNoTrailing HTTP endpoint.
func SlashWithBasePathNoTrailingBasePathNoTrailingPath() string {
return "/foo"
}
`

var PathOneParamCode = `// MethodPathOneParamServicePathOneParamPath returns the URL path to the ServicePathOneParam service MethodPathOneParam HTTP endpoint.
func MethodPathOneParamServicePathOneParamPath(a string) string {
return fmt.Sprintf("/one/%v/two", a)
var BasePathNoTrailing_TrailingWithBasePathNoTrailingCode = `// TrailingWithBasePathNoTrailingBasePathNoTrailingPath returns the URL path to the BasePathNoTrailing service TrailingWithBasePathNoTrailing HTTP endpoint.
func TrailingWithBasePathNoTrailingBasePathNoTrailingPath() string {
return "/foo/bar/"
}
`

var BasePathWithTrailingSlash_WithBasePathWithTrailingCode = `// SlashWithBasePathWithTrailingBasePathWithTrailingPath returns the URL path to the BasePathWithTrailing service SlashWithBasePathWithTrailing HTTP endpoint.
func SlashWithBasePathWithTrailingBasePathWithTrailingPath() string {
return "/foo/"
}
`

var NoBasePath_SlashNoBasePathCode = `// SlashNoBasePathNoBasePathPath returns the URL path to the NoBasePath service SlashNoBasePath HTTP endpoint.
func SlashNoBasePathNoBasePathPath() string {
return "/"
}
`

var PathOneParamTrailingSlashCode = `// MethodPathOneParamTrailingSlashServicePathOneParamTrailingSlashPath returns the URL path to the ServicePathOneParamTrailingSlash service MethodPathOneParamTrailingSlash HTTP endpoint.
func MethodPathOneParamTrailingSlashServicePathOneParamTrailingSlashPath(a string) string {
return fmt.Sprintf("/one/%v/", a)
var NoBasePath_TrailingNoBasePathCode = `// TrailingNoBasePathNoBasePathPath returns the URL path to the NoBasePath service TrailingNoBasePath HTTP endpoint.
func TrailingNoBasePathNoBasePathPath() string {
return "/foo/"
}
`

var BasePath_SpecialTrailingSlashCode = `// SpecialTrailingSlashBasePathPath returns the URL path to the BasePath service SpecialTrailingSlash HTTP endpoint.
func SpecialTrailingSlashBasePathPath() string {
return "/foo/"
}
`

var PathOneParamCode = `// MethodPathOneParamServicePathOneParamPath returns the URL path to the ServicePathOneParam service MethodPathOneParam HTTP endpoint.
func MethodPathOneParamServicePathOneParamPath(a string) string {
return fmt.Sprintf("/one/%v/two", a)
}
`

Expand Down
2 changes: 1 addition & 1 deletion http/codegen/testdata/server_init_functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func New(
return &Server{
Mounts: []*MountPoint{
{"MethodMultiEndpoints1", "GET", "/server_multi_endpoints/{id}"},
{"MethodMultiEndpoints2", "POST", "/server_multi_endpoints/"},
{"MethodMultiEndpoints2", "POST", "/server_multi_endpoints"},
},
MethodMultiEndpoints1: NewMethodMultiEndpoints1Handler(e.MethodMultiEndpoints1, mux, decoder, encoder, errhandler, formatter),
MethodMultiEndpoints2: NewMethodMultiEndpoints2Handler(e.MethodMultiEndpoints2, mux, decoder, encoder, errhandler, formatter),
Expand Down

0 comments on commit 191d509

Please sign in to comment.