Skip to content

Commit

Permalink
Use json camel case when GetUseJSONNamesForFields is enabled (#985)
Browse files Browse the repository at this point in the history 
* Use json name when GetUseJSONNamesForFields is enabled

* Optimzed an if statement

* Merge two if statements

* Convert parameters to lower camel case in url

* Revert services.go file

* Fix an unit test

* Update bazel file

* Add an unit test for testing json camel case

* Add an unit test for testing code without enabling json camel case

* Add more corner cases

Fixes #986
  • Loading branch information
@xin-au @johanbrandhorst
xin-au authored and johanbrandhorst committed Aug 9, 2019
1 parent fdf0635 commit 643a3cf
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 9 deletions.
1 change: 1 addition & 0 deletions protoc-gen-swagger/genswagger/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ go_library(
"@com_github_golang_glog//:go_default_library",
"@com_github_golang_protobuf//descriptor:go_default_library_gen",
"@com_github_golang_protobuf//proto:go_default_library",
"@com_github_golang_protobuf//protoc-gen-go/generator:go_default_library_gen",
"@io_bazel_rules_go//proto/wkt:any_go_proto",
"@io_bazel_rules_go//proto/wkt:compiler_plugin_go_proto",
"@io_bazel_rules_go//proto/wkt:descriptor_go_proto",
Expand Down
34 changes: 29 additions & 5 deletions protoc-gen-swagger/genswagger/template.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/golang/glog"
"github.com/golang/protobuf/proto"
pbdescriptor "github.com/golang/protobuf/protoc-gen-go/descriptor"
gogen "github.com/golang/protobuf/protoc-gen-go/generator"
"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 @@ -597,7 +598,7 @@ func resolveFullyQualifiedNameToSwaggerNames(messages []string, useFQNForSwagger
}

// 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) string {
func templateToSwaggerPath(path string, reg *descriptor.Registry) 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 All @@ -606,12 +607,15 @@ func templateToSwaggerPath(path string) string {
var parts []string
depth := 0
buffer := ""
jsonBuffer := ""
for _, char := range path {
switch char {
case '{':
// Push on the stack
depth++
buffer += string(char)
jsonBuffer = ""
jsonBuffer += string(char)
break
case '}':
if depth == 0 {
Expand All @@ -620,6 +624,14 @@ func templateToSwaggerPath(path string) string {
// Pop from the stack
depth--
buffer += string(char)
if reg.GetUseJSONNamesForFields() &&
len(jsonBuffer) > 1 {
jsonSnakeCaseName := string(jsonBuffer[1 : len(buffer)-1])
jsonCamelCaseName := string(lowerCamelCase(jsonSnakeCaseName))
prev := string(buffer[:len(buffer)-len(jsonSnakeCaseName)-2])
buffer = strings.Join([]string{prev, "{", jsonCamelCaseName, "}"}, "")
jsonBuffer = ""
}
case '/':
if depth == 0 {
parts = append(parts, buffer)
Expand All @@ -631,6 +643,7 @@ func templateToSwaggerPath(path string) string {
buffer += string(char)
default:
buffer += string(char)
jsonBuffer += string(char)
break
}
}
Expand Down Expand Up @@ -731,9 +744,12 @@ func renderServices(services []*descriptor.Service, paths swaggerPathsObject, re
if desc == "" {
desc = fieldProtoComments(reg, parameter.Target.Message, parameter.Target)
}

parameterString := parameter.String()
if reg.GetUseJSONNamesForFields() {
parameterString = lowerCamelCase(parameterString)
}
parameters = append(parameters, swaggerParameterObject{
Name: parameter.String(),
Name: parameterString,
Description: desc,
In: "path",
Required: true,
Expand Down Expand Up @@ -797,7 +813,7 @@ func renderServices(services []*descriptor.Service, paths swaggerPathsObject, re
parameters = append(parameters, queryParams...)
}

pathItemObject, ok := paths[templateToSwaggerPath(b.PathTmpl.Template)]
pathItemObject, ok := paths[templateToSwaggerPath(b.PathTmpl.Template, reg)]
if !ok {
pathItemObject = swaggerPathItemObject{}
}
Expand Down Expand Up @@ -947,7 +963,7 @@ func renderServices(services []*descriptor.Service, paths swaggerPathsObject, re
pathItemObject.Patch = operationObject
break
}
paths[templateToSwaggerPath(b.PathTmpl.Template)] = pathItemObject
paths[templateToSwaggerPath(b.PathTmpl.Template, reg)] = pathItemObject
}
}
}
Expand Down Expand Up @@ -1661,3 +1677,11 @@ func addCustomRefs(d swaggerDefinitionsObject, reg *descriptor.Registry, refs re
// Run again in case any new refs were added
addCustomRefs(d, reg, refs)
}

func lowerCamelCase(parameter string) string {
parameterString := gogen.CamelCase(parameter)
builder := &strings.Builder{}
builder.WriteString(strings.ToLower(string(parameterString[0])))
builder.WriteString(parameterString[1:])
return builder.String()
}
61 changes: 57 additions & 4 deletions protoc-gen-swagger/genswagger/template_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,59 @@ func TestApplyTemplateRequestWithUnusedReferences(t *testing.T) {
}
}

func TestTemplateWithJsonCamelCase(t *testing.T) {
var tests = []struct {
input string
expected string
}{
{"/test/{test_id}", "/test/{testId}"},
{"/test1/{test1_id}/test2/{test2_id}", "/test1/{test1Id}/test2/{test2Id}"},
{"/test1/{test1_id}/{test2_id}", "/test1/{test1Id}/{test2Id}"},
{"/test1/test2/{test1_id}/{test2_id}", "/test1/test2/{test1Id}/{test2Id}"},
{"/test1/{test1_id1_id2}", "/test1/{test1Id1Id2}"},
{"/test1/{test1_id1_id2}/test2/{test2_id3_id4}", "/test1/{test1Id1Id2}/test2/{test2Id3Id4}"},
{"/test1/test2/{test1_id1_id2}/{test2_id3_id4}", "/test1/test2/{test1Id1Id2}/{test2Id3Id4}"},
{"test/{a}", "test/{a}"},
{"test/{ab}", "test/{ab}"},
{"test/{a_a}", "test/{aA}"},
{"test/{ab_c}", "test/{abC}"},
}
reg := descriptor.NewRegistry()
reg.SetUseJSONNamesForFields(true)
for _, data := range tests {
actual := templateToSwaggerPath(data.input, reg)
if data.expected != actual {
t.Errorf("Expected templateToSwaggerPath(%v) = %v, actual: %v", data.input, data.expected, actual)
}
}
}

func TestTemplateWithoutJsonCamelCase(t *testing.T) {
var tests = []struct {
input string
expected string
}{
{"/test/{test_id}", "/test/{test_id}"},
{"/test1/{test1_id}/test2/{test2_id}", "/test1/{test1_id}/test2/{test2_id}"},
{"/test1/{test1_id}/{test2_id}", "/test1/{test1_id}/{test2_id}"},
{"/test1/test2/{test1_id}/{test2_id}", "/test1/test2/{test1_id}/{test2_id}"},
{"/test1/{test1_id1_id2}", "/test1/{test1_id1_id2}"},
{"/test1/{test1_id1_id2}/test2/{test2_id3_id4}", "/test1/{test1_id1_id2}/test2/{test2_id3_id4}"},
{"/test1/test2/{test1_id1_id2}/{test2_id3_id4}", "/test1/test2/{test1_id1_id2}/{test2_id3_id4}"},
{"test/{a}", "test/{a}"},
{"test/{ab}", "test/{ab}"},
{"test/{a_a}", "test/{a_a}"},
}
reg := descriptor.NewRegistry()
reg.SetUseJSONNamesForFields(false)
for _, data := range tests {
actual := templateToSwaggerPath(data.input, reg)
if data.expected != actual {
t.Errorf("Expected templateToSwaggerPath(%v) = %v, actual: %v", data.input, data.expected, actual)
}
}
}

func TestTemplateToSwaggerPath(t *testing.T) {
var tests = []struct {
input string
Expand All @@ -860,9 +913,9 @@ func TestTemplateToSwaggerPath(t *testing.T) {
{"/{user.name=prefix1/*/prefix2/*}:customMethod", "/{user.name=prefix1/*/prefix2/*}:customMethod"},
{"/{parent=prefix/*}/children:customMethod", "/{parent=prefix/*}/children:customMethod"},
}

reg := descriptor.NewRegistry()
for _, data := range tests {
actual := templateToSwaggerPath(data.input)
actual := templateToSwaggerPath(data.input, reg)
if data.expected != actual {
t.Errorf("Expected templateToSwaggerPath(%v) = %v, actual: %v", data.input, data.expected, actual)
}
Expand Down Expand Up @@ -937,9 +990,9 @@ func TestFQMNtoSwaggerName(t *testing.T) {
{"/{test1}/{test2}", "/{test1}/{test2}"},
{"/{test1}/{test2}/", "/{test1}/{test2}/"},
}

reg := descriptor.NewRegistry()
for _, data := range tests {
actual := templateToSwaggerPath(data.input)
actual := templateToSwaggerPath(data.input, reg)
if data.expected != actual {
t.Errorf("Expected templateToSwaggerPath(%v) = %v, actual: %v", data.input, data.expected, actual)
}
Expand Down

0 comments on commit 643a3cf

Please sign in to comment.