Skip to content

Commit

Permalink
Revert "Fix templateToSwaggerPath generates invalid path"
Browse files Browse the repository at this point in the history
This reverts commit 6966912.

This commit was causing panics for some valid paths.
  • Loading branch information
johanbrandhorst committed Nov 6, 2019
1 parent da7a886 commit 6cf5d51
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 43 deletions.
35 changes: 3 additions & 32 deletions protoc-gen-swagger/genswagger/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"encoding/json"
"fmt"
"io/ioutil"
"net/url"
"os"
"reflect"
"regexp"
Expand All @@ -18,9 +17,9 @@ import (
"github.com/golang/glog"
"github.com/golang/protobuf/jsonpb"
"github.com/golang/protobuf/proto"
structpb "github.com/golang/protobuf/ptypes/struct"
pbdescriptor "github.com/golang/protobuf/protoc-gen-go/descriptor"
gogen "github.com/golang/protobuf/protoc-gen-go/generator"
structpb "github.com/golang/protobuf/ptypes/struct"
"github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway/descriptor"
swagger_options "github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger/options"
)
Expand Down Expand Up @@ -679,18 +678,13 @@ func templateToSwaggerPath(path string, reg *descriptor.Registry) string {
// Parts is now an array of segments of the path. Interestingly, since the
// syntax for this subsection CAN be handled by a regexp since it has no
// memory.
keyre := regexp.MustCompile("{(.*)}")
for index, part := range parts {
// If part is a resource name such as "parent", "name", "user.name", the format info must be retained.
prefix := canRegexp.ReplaceAllString(part, "$1")
if isResourceName(prefix) {
sm := keyre.FindStringSubmatch(part)
key := sm[1]
esckey := url.PathEscape(key)
parts[index] = keyre.ReplaceAllString(part, fmt.Sprintf("{%s}", esckey))
} else {
parts[index] = canRegexp.ReplaceAllString(part, "{$1}")
continue
}
parts[index] = canRegexp.ReplaceAllString(part, "{$1}")
}

return strings.Join(parts, "/")
Expand All @@ -705,31 +699,11 @@ func isResourceName(prefix string) bool {
return field == "parent" || field == "name"
}

func extractResourceName(path string) map[string]string {
m := map[string]string{}
keyre := regexp.MustCompile("{(.*)}")
sm := keyre.FindStringSubmatch(path)
count := len(sm)
for i := 0; i < count; i++ {
key := sm[1]
parts := strings.Split(key, "=")
label := parts[0]
parts = strings.Split(label, ".")
l := len(parts)
field := parts[l-1]
if field == "parent" || field == "name" {
m[label] = key
}
}
return m
}

func renderServices(services []*descriptor.Service, paths swaggerPathsObject, reg *descriptor.Registry, requestResponseRefs, customRefs refMap) error {
// Correctness of svcIdx and methIdx depends on 'services' containing the services in the same order as the 'file.Service' array.
for svcIdx, svc := range services {
for methIdx, meth := range svc.Methods {
for bIdx, b := range meth.Bindings {
pathParamMap := extractResourceName(templateToSwaggerPath(b.PathTmpl.Template, reg))
// Iterate over all the swagger parameters
parameters := swaggerParametersObject{}
for _, parameter := range b.PathParams {
Expand Down Expand Up @@ -797,9 +771,6 @@ func renderServices(services []*descriptor.Service, paths swaggerPathsObject, re
if reg.GetUseJSONNamesForFields() {
parameterString = lowerCamelCase(parameterString)
}
if esckey, ok := pathParamMap[parameterString]; ok {
parameterString = esckey
}
parameters = append(parameters, swaggerParameterObject{
Name: parameterString,
Description: desc,
Expand Down
21 changes: 10 additions & 11 deletions protoc-gen-swagger/genswagger/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1050,17 +1050,16 @@ func TestTemplateToSwaggerPath(t *testing.T) {
{"/{test=prefix/that/has/multiple/parts/to/it/*}", "/{test}"},
{"/{test1}/{test2}", "/{test1}/{test2}"},
{"/{test1}/{test2}/", "/{test1}/{test2}/"},
{"/{name=prefix/*}", "/{name=prefix%2F%2A}"},
{"/{name=prefix1/*/prefix2/*}", "/{name=prefix1%2F%2A%2Fprefix2%2F%2A}"},
{"/{parent=prefix1/*}/{name=prefix2/*}", "/{parent=prefix1%2F%2A}/{name=prefix2%2F%2A}"},
{"/{user.name=prefix/*}", "/{user.name=prefix%2F%2A}"},
{"/{user.name=prefix1/*/prefix2/*}", "/{user.name=prefix1%2F%2A%2Fprefix2%2F%2A}"},
{"/{parent=prefix/*}/children", "/{parent=prefix%2F%2A}/children"},
{"/{name=prefix/*}:customMethod", "/{name=prefix%2F%2A}:customMethod"},
{"/{name=prefix1/*/prefix2/*}:customMethod", "/{name=prefix1%2F%2A%2Fprefix2%2F%2A}:customMethod"},
{"/{user.name=prefix/*}:customMethod", "/{user.name=prefix%2F%2A}:customMethod"},
{"/{user.name=prefix1/*/prefix2/*}:customMethod", "/{user.name=prefix1%2F%2A%2Fprefix2%2F%2A}:customMethod"},
{"/{parent=prefix/*}/children:customMethod", "/{parent=prefix%2F%2A}/children:customMethod"},
{"/{name=prefix/*}", "/{name=prefix/*}"},
{"/{name=prefix1/*/prefix2/*}", "/{name=prefix1/*/prefix2/*}"},
{"/{user.name=prefix/*}", "/{user.name=prefix/*}"},
{"/{user.name=prefix1/*/prefix2/*}", "/{user.name=prefix1/*/prefix2/*}"},
{"/{parent=prefix/*}/children", "/{parent=prefix/*}/children"},
{"/{name=prefix/*}:customMethod", "/{name=prefix/*}:customMethod"},
{"/{name=prefix1/*/prefix2/*}:customMethod", "/{name=prefix1/*/prefix2/*}:customMethod"},
{"/{user.name=prefix/*}:customMethod", "/{user.name=prefix/*}:customMethod"},
{"/{user.name=prefix1/*/prefix2/*}:customMethod", "/{user.name=prefix1/*/prefix2/*}:customMethod"},
{"/{parent=prefix/*}/children:customMethod", "/{parent=prefix/*}/children:customMethod"},
}
reg := descriptor.NewRegistry()
reg.SetUseJSONNamesForFields(false)
Expand Down

0 comments on commit 6cf5d51

Please sign in to comment.