Skip to content

Commit

Permalink
Support reserved json name and add tests (#1085)
Browse files Browse the repository at this point in the history
* Support reserved json name and add tests

* Correct some variable names

* Optimize a logic for assigning a reserved json name to jsonCamelCaseName

* Put a logic for checking if there is a reseved json name in the method of lowerCamelCase

Fixes #1084
  • Loading branch information
xin-au authored and johanbrandhorst committed Nov 14, 2019
1 parent f712043 commit 9087bb8
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 16 deletions.
21 changes: 13 additions & 8 deletions protoc-gen-swagger/genswagger/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,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 @@ -621,7 +621,7 @@ func resolveFullyQualifiedNameToSwaggerNames(messages []string, useFQNForSwagger
var canRegexp = regexp.MustCompile("{([a-zA-Z][a-zA-Z0-9_.]*).*}")

// Swagger expects paths of the form /path/{string_value} but grpc-gateway paths are expected to be of the form /path/{string_value=strprefix/*}. This should reformat it correctly.
func templateToSwaggerPath(path string, reg *descriptor.Registry) string {
func templateToSwaggerPath(path string, reg *descriptor.Registry, fields []*descriptor.Field) string {
// It seems like the right thing to do here is to just use
// strings.Split(path, "/") but that breaks badly when you hit a url like
// /{my_field=prefix/*}/ and end up with 2 sections representing my_field.
Expand Down Expand Up @@ -650,7 +650,7 @@ func templateToSwaggerPath(path string, reg *descriptor.Registry) string {
if reg.GetUseJSONNamesForFields() &&
len(jsonBuffer) > 1 {
jsonSnakeCaseName := string(jsonBuffer[1:])
jsonCamelCaseName := string(lowerCamelCase(jsonSnakeCaseName))
jsonCamelCaseName := string(lowerCamelCase(jsonSnakeCaseName, fields))
prev := string(buffer[:len(buffer)-len(jsonSnakeCaseName)-2])
buffer = strings.Join([]string{prev, "{", jsonCamelCaseName, "}"}, "")
jsonBuffer = ""
Expand Down Expand Up @@ -769,7 +769,7 @@ func renderServices(services []*descriptor.Service, paths swaggerPathsObject, re
}
parameterString := parameter.String()
if reg.GetUseJSONNamesForFields() {
parameterString = lowerCamelCase(parameterString)
parameterString = lowerCamelCase(parameterString, meth.RequestType.Fields)
}
parameters = append(parameters, swaggerParameterObject{
Name: parameterString,
Expand Down Expand Up @@ -836,7 +836,7 @@ func renderServices(services []*descriptor.Service, paths swaggerPathsObject, re
parameters = append(parameters, queryParams...)
}

pathItemObject, ok := paths[templateToSwaggerPath(b.PathTmpl.Template, reg)]
pathItemObject, ok := paths[templateToSwaggerPath(b.PathTmpl.Template, reg, meth.RequestType.Fields)]
if !ok {
pathItemObject = swaggerPathItemObject{}
}
Expand Down Expand Up @@ -1002,7 +1002,7 @@ func renderServices(services []*descriptor.Service, paths swaggerPathsObject, re
pathItemObject.Patch = operationObject
break
}
paths[templateToSwaggerPath(b.PathTmpl.Template, reg)] = pathItemObject
paths[templateToSwaggerPath(b.PathTmpl.Template, reg, meth.RequestType.Fields)] = pathItemObject
}
}
}
Expand Down Expand Up @@ -1802,8 +1802,13 @@ func addCustomRefs(d swaggerDefinitionsObject, reg *descriptor.Registry, refs re
addCustomRefs(d, reg, refs)
}

func lowerCamelCase(parameter string) string {
parameterString := gogen.CamelCase(parameter)
func lowerCamelCase(fieldName string, fields []*descriptor.Field) string {
for _, oneField := range fields {
if oneField.GetName() == fieldName {
return oneField.GetJsonName()
}
}
parameterString := gogen.CamelCase(fieldName)
builder := &strings.Builder{}
builder.WriteString(strings.ToLower(string(parameterString[0])))
builder.WriteString(parameterString[1:])
Expand Down
27 changes: 19 additions & 8 deletions protoc-gen-swagger/genswagger/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,15 @@ func TestApplyTemplateRequestWithUnusedReferences(t *testing.T) {
}
}

func generateFieldsForJSONReservedName() []*descriptor.Field {
fields := make([]*descriptor.Field, 0)
fieldName := string("json_name")
fieldJSONName := string("jsonNAME")
fieldDescriptor := protodescriptor.FieldDescriptorProto{Name: &fieldName, JsonName: &fieldJSONName}
field := &descriptor.Field{FieldDescriptorProto: &fieldDescriptor}
return append(fields, field)
}

func TestTemplateWithJsonCamelCase(t *testing.T) {
var tests = []struct {
input string
Expand All @@ -1002,11 +1011,12 @@ func TestTemplateWithJsonCamelCase(t *testing.T) {
{"test/{ab}", "test/{ab}"},
{"test/{a_a}", "test/{aA}"},
{"test/{ab_c}", "test/{abC}"},
{"test/{json_name}", "test/{jsonNAME}"},
}
reg := descriptor.NewRegistry()
reg.SetUseJSONNamesForFields(true)
for _, data := range tests {
actual := templateToSwaggerPath(data.input, reg)
actual := templateToSwaggerPath(data.input, reg, generateFieldsForJSONReservedName())
if data.expected != actual {
t.Errorf("Expected templateToSwaggerPath(%v) = %v, actual: %v", data.input, data.expected, actual)
}
Expand All @@ -1028,11 +1038,12 @@ func TestTemplateWithoutJsonCamelCase(t *testing.T) {
{"test/{a}", "test/{a}"},
{"test/{ab}", "test/{ab}"},
{"test/{a_a}", "test/{a_a}"},
{"test/{json_name}", "test/{json_name}"},
}
reg := descriptor.NewRegistry()
reg.SetUseJSONNamesForFields(false)
for _, data := range tests {
actual := templateToSwaggerPath(data.input, reg)
actual := templateToSwaggerPath(data.input, reg, generateFieldsForJSONReservedName())
if data.expected != actual {
t.Errorf("Expected templateToSwaggerPath(%v) = %v, actual: %v", data.input, data.expected, actual)
}
Expand Down Expand Up @@ -1064,14 +1075,14 @@ func TestTemplateToSwaggerPath(t *testing.T) {
reg := descriptor.NewRegistry()
reg.SetUseJSONNamesForFields(false)
for _, data := range tests {
actual := templateToSwaggerPath(data.input, reg)
actual := templateToSwaggerPath(data.input, reg, generateFieldsForJSONReservedName())
if data.expected != actual {
t.Errorf("Expected templateToSwaggerPath(%v) = %v, actual: %v", data.input, data.expected, actual)
}
}
reg.SetUseJSONNamesForFields(true)
for _, data := range tests {
actual := templateToSwaggerPath(data.input, reg)
actual := templateToSwaggerPath(data.input, reg, generateFieldsForJSONReservedName())
if data.expected != actual {
t.Errorf("Expected templateToSwaggerPath(%v) = %v, actual: %v", data.input, data.expected, actual)
}
Expand All @@ -1086,7 +1097,7 @@ func BenchmarkTemplateToSwaggerPath(b *testing.B) {
reg.SetUseJSONNamesForFields(false)

for i := 0; i < b.N; i++ {
_ = templateToSwaggerPath(input, reg)
_ = templateToSwaggerPath(input, reg, generateFieldsForJSONReservedName())
}
})

Expand All @@ -1095,7 +1106,7 @@ func BenchmarkTemplateToSwaggerPath(b *testing.B) {
reg.SetUseJSONNamesForFields(true)

for i := 0; i < b.N; i++ {
_ = templateToSwaggerPath(input, reg)
_ = templateToSwaggerPath(input, reg, generateFieldsForJSONReservedName())
}
})
}
Expand Down Expand Up @@ -1171,14 +1182,14 @@ func TestFQMNtoSwaggerName(t *testing.T) {
reg := descriptor.NewRegistry()
reg.SetUseJSONNamesForFields(false)
for _, data := range tests {
actual := templateToSwaggerPath(data.input, reg)
actual := templateToSwaggerPath(data.input, reg, generateFieldsForJSONReservedName())
if data.expected != actual {
t.Errorf("Expected templateToSwaggerPath(%v) = %v, actual: %v", data.input, data.expected, actual)
}
}
reg.SetUseJSONNamesForFields(true)
for _, data := range tests {
actual := templateToSwaggerPath(data.input, reg)
actual := templateToSwaggerPath(data.input, reg, generateFieldsForJSONReservedName())
if data.expected != actual {
t.Errorf("Expected templateToSwaggerPath(%v) = %v, actual: %v", data.input, data.expected, actual)
}
Expand Down

0 comments on commit 9087bb8

Please sign in to comment.