Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

protoc-gen-openapiv2: Use the canonical camelCase converter for protobuf #2599

Merged
merged 2 commits into from
Mar 21, 2022
Merged

protoc-gen-openapiv2: Use the canonical camelCase converter for protobuf #2599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d6949db
Use the canonical camelCase converter for protobuf. Fixes issue #2363.
oyvindwe Mar 21, 2022
3560f03
Revert change that should go in separate PR
oyvindwe Mar 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/casing/LICENSE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Copyright 2010 The Go Authors. All rights reserved.
Copyright 2010, 2019 The Go Authors. All rights reserved.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are
Expand Down
8 changes: 5 additions & 3 deletions internal/casing/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Case conversion

This package contains a single function, copied from the
`github.com/golang/protobuf/protoc-gen-go/generator` package. That
modules LICENSE is referenced in its entirety in this package.
This package contains two functions:
- `Camel` copied from the `github.com/golang/protobuf/protoc-gen-go/generator` package.
- `JSONCamelCase` copied from the `github.com/protocolbuffers/protobuf-go/internal/strs` package.

Both these modules are licensed by The Go Authors, as reflected in this package's [LICENSE.md].
18 changes: 18 additions & 0 deletions internal/casing/camel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,24 @@ func Camel(s string) string {
return string(t)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, do we still need the old function? We should probably always use the new one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the old one - that was coded inline in template.go.

The Camel method looks more like GoCamel here though: https://github.com/protocolbuffers/protobuf-go/blob/3992ea83a23c00882339f33511074d251e19822c/internal/strs/strings.go#L28-L32

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect their logic to be roughly the same, though it might be worth using the new one anyway. We should probably always be consistent with what APIv2 uses (the new repo).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if I follow you here. Do you mean that Camel should be replaced with GoCamelCase? That method seems to now only be called from protoc-gen-grpc-gateway, which I'm not familiar with (we use ESP).

What do you refer to with "APIv2 uses (the new repo)"?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to make any changes here, but there may be a subtle bug hidden somewhere here that will eventually rear its head. By APIv2 I mean the Go protobuf stack in google.golang.org/protobuf. That's the "new" stack. See https://blog.golang.org/protobuf-apiv2 for more information. The grpc-gateway v2 is built on APIv2.


// JSONCamelCase converts a snake_case identifier to a camelCase identifier,
// according to the protobuf JSON specification.
func JSONCamelCase(s string) string {
var b []byte
var wasUnderscore bool
for i := 0; i < len(s); i++ { // proto identifiers are always ASCII
c := s[i]
if c != '_' {
if wasUnderscore && isASCIILower(c) {
c -= 'a' - 'A' // convert to uppercase
}
b = append(b, c)
}
wasUnderscore = c == '_'
}
return string(b)
}

// And now lots of helper functions.

// Is c an ASCII lower-case letter?
Expand Down
14 changes: 3 additions & 11 deletions protoc-gen-openapiv2/internal/genopenapi/template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2523,7 +2523,7 @@ func updateswaggerObjectFromJSONSchema(s *openapiSchemaObject, j *openapi_option
if reg.GetUseJSONNamesForFields() {
for i, r := range s.Required {
// TODO(oyvindwe): Look up field and use field.GetJsonName()?
s.Required[i] = doCamelCase(r)
s.Required[i] = casing.JSONCamelCase(r)
}
}
s.Enum = j.GetEnum()
Expand Down Expand Up @@ -2707,23 +2707,15 @@ func lowerCamelCase(fieldName string, fields []*descriptor.Field, msgs []*descri
fieldNames := strings.Split(fieldName, ".")
fieldNamesWithCamelCase := make([]string, 0)
for i := 0; i < len(fieldNames)-1; i++ {
fieldNamesWithCamelCase = append(fieldNamesWithCamelCase, doCamelCase(string(fieldNames[i])))
fieldNamesWithCamelCase = append(fieldNamesWithCamelCase, casing.JSONCamelCase(string(fieldNames[i])))
}
prefix := strings.Join(fieldNamesWithCamelCase, ".")
reservedJSONName := getReservedJSONName(fieldName, messageNameToFieldsToJSONName, fieldNameToType)
if reservedJSONName != "" {
return prefix + "." + reservedJSONName
}
}
return doCamelCase(fieldName)
}

func doCamelCase(input string) string {
parameterString := casing.Camel(input)
builder := &strings.Builder{}
builder.WriteString(strings.ToLower(string(parameterString[0])))
builder.WriteString(parameterString[1:])
return builder.String()
return casing.JSONCamelCase(fieldName)
}

func getReservedJSONName(fieldName string, messageNameToFieldsToJSONName map[string]map[string]string, fieldNameToType map[string]string) string {
Expand Down