fix(generator): use json_name when available#12940
fix(generator): use json_name when available#12940diegomarquezp wants to merge 7 commits intomainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for the json_name protobuf option within the GAPIC generator for Java REST clients, ensuring that HTTP query parameters correctly use custom JSON names, such as those in kebab-case. The changes involve updating the Field and HttpBinding models to store the JSON name and modifying the HttpRuleParser to extract and propagate this information. Review feedback highlights that certain ternary checks for jsonName are redundant because the protobuf descriptor provides a default value, and suggests using the diamond operator for ArrayList instantiation to avoid raw types.
I am having trouble creating individual review comments. Click here to see my feedback.
sdk-platform-java/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceStubClassComposer.java (929-931)
The fieldDescriptor.getJsonName() method (used in Parser.java to populate the Field model) always returns a non-empty string, as it defaults to the lower-camel-case version of the field name if the json_name option is not explicitly set in the proto. Consequently, httpBindingFieldName.jsonName() is unlikely to be null here, making this ternary operation and the fallback to JavaStyle.toLowerCamelCase redundant. If the intention is to use the generator's custom camel-casing logic for fields without an explicit json_name, the parser should be updated to only set jsonName when the option is present.
References
- Avoid adding defensive null checks for values that are guaranteed to be non-null by design, as this can hide invariant breaks.
sdk-platform-java/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java (150)
Avoid using raw types. Use the diamond operator <> for the ArrayList instantiation.
List<String> jsonNameParts = new ArrayList<>();
sdk-platform-java/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java (163-166)
As noted in other files, field.jsonName() is expected to be non-null and non-empty because it is populated from fieldDescriptor.getJsonName(), which provides a default camel-case name if the json_name option is missing. This ternary check and the fallback to JavaStyle.toLowerCamelCase appear to be unreachable code under the current parsing logic.
References
- Avoid adding defensive null checks for values that are guaranteed to be non-null by design, as this can hide invariant breaks.
|
|
blakeli0
left a comment
There was a problem hiding this comment.
Pending discussion of b/505851871#comment4





b/505851871