Skip to content

Commit 622955b

Browse files
fix(generator): use json_name when available (#12940)
b/505851871 Demo in #12968
1 parent 522e05b commit 622955b

16 files changed

Lines changed: 674 additions & 14 deletions

File tree

.gitignore

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ nosetests.xml
5858
.settings
5959
.DS_Store
6060
.classpath
61+
.vscode
6162

6263
# API key file containing value of GOOGLE_API_KEY for integration tests
6364
api_key
@@ -79,4 +80,4 @@ monorepo
7980
*.tfstate.*.backup
8081
*.tfstate.lock.info
8182

82-
.jqwik-database
83+
.jqwik-database

sdk-platform-java/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceStubClassComposer.java

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
import com.google.api.generator.gapic.model.Service;
6363
import com.google.api.generator.gapic.utils.JavaStyle;
6464
import com.google.common.annotations.VisibleForTesting;
65+
import com.google.common.base.Strings;
6566
import com.google.common.collect.BiMap;
6667
import com.google.common.collect.ImmutableList;
6768
import com.google.common.collect.ImmutableMap;
@@ -763,7 +764,16 @@ private Expr createBodyFieldsExtractorClassInstance(
763764
// Handle foo.bar cases by descending into the subfields.
764765
MethodInvocationExpr.Builder requestFieldMethodExprBuilder =
765766
MethodInvocationExpr.builder().setExprReferenceExpr(prevExpr);
766-
bodyParamName = JavaStyle.toLowerCamelCase(httpBindingFieldName.name());
767+
// Use explicit json_name if defined in the proto, prioritizing the actual wire name
768+
// over Java-escaped identifiers. Note that trailing underscores (e.g., 'case_') result from:
769+
// 1. protoc-gen-java:
770+
// https://github.com/protocolbuffers/protobuf/blob/cecbbf41e43634c7c5b940dd336aa81b31fd4e5d/src/google/protobuf/compiler/java/names.cc#L189-L195
771+
// 2. gapic-generator-java Keyword implementation:
772+
// com/google/api/generator/engine/lexicon/Keyword.java#L92-L94
773+
bodyParamName =
774+
!Strings.isNullOrEmpty(httpBindingFieldName.jsonName())
775+
? httpBindingFieldName.jsonName()
776+
: JavaStyle.toLowerCamelCase(httpBindingFieldName.name());
767777
String[] descendantFields = httpBindingFieldName.name().split("\\.");
768778
if (asteriskBody && descendantFields.length > 1) {
769779
// This is the `body: "*"` case, do not clean nested body fields as it a very rare, not
@@ -926,7 +936,12 @@ private Expr createFieldsExtractorClassInstance(
926936
paramsPutArgs.add(
927937
ValueExpr.withValue(
928938
StringObjectValue.withValue(
929-
JavaStyle.toLowerCamelCase(httpBindingFieldName.name()))));
939+
// Use explicit json_name if defined in the proto, prioritizing the actual wire
940+
// name over Java-escaped identifiers (e.g., avoiding 'case_' generated to prevent
941+
// keywords conflict).
942+
(httpBindingFieldName.jsonName() != null)
943+
? httpBindingFieldName.jsonName()
944+
: JavaStyle.toLowerCamelCase(httpBindingFieldName.name()))));
930945
paramsPutArgs.add(requestBuilderExpr);
931946

932947
Expr paramsPutExpr =

sdk-platform-java/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ public abstract class Field {
3131
// resolution behavior. For more context, please see the invocation site of the setter method.
3232
public abstract String originalName();
3333

34+
@Nullable
35+
public abstract String jsonName();
36+
3437
public abstract TypeNode type();
3538

3639
// If the field is annotated with google.api.field_behavior = REQUIRED, then this is true. This is
@@ -92,6 +95,7 @@ public boolean equals(Object o) {
9295
Field other = (Field) o;
9396
return name().equals(other.name())
9497
&& originalName().equals(other.originalName())
98+
&& Objects.equals(jsonName(), other.jsonName())
9599
&& type().equals(other.type())
96100
&& isRequired() == other.isRequired()
97101
&& fieldInfoFormat() == other.fieldInfoFormat()
@@ -109,6 +113,7 @@ && isProto3Optional() == other.isProto3Optional()
109113
public int hashCode() {
110114
return 17 * name().hashCode()
111115
+ 31 * originalName().hashCode()
116+
+ (jsonName() == null ? 0 : jsonName().hashCode())
112117
+ 19 * type().hashCode()
113118
+ (isMessage() ? 1 : 0) * 23
114119
+ (isEnum() ? 1 : 0) * 29
@@ -141,6 +146,8 @@ public abstract static class Builder {
141146

142147
public abstract Builder setOriginalName(String originalName);
143148

149+
public abstract Builder setJsonName(String jsonName);
150+
144151
public abstract Builder setType(TypeNode type);
145152

146153
public abstract Builder setIsRequired(boolean isRequired);

sdk-platform-java/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/HttpBindings.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import com.google.api.generator.gapic.utils.JavaStyle;
1818
import com.google.auto.value.AutoValue;
19+
import com.google.common.base.Strings;
1920
import com.google.common.collect.ImmutableSet;
2021
import java.util.LinkedHashMap;
2122
import java.util.List;
@@ -42,6 +43,11 @@ public abstract static class HttpBinding implements Comparable<HttpBinding> {
4243

4344
abstract String lowerCamelName();
4445

46+
// The dot-separated json_name of the field.
47+
// e.g. parent.iceberg-catalog-id
48+
@Nullable
49+
public abstract String jsonName();
50+
4551
// An object that contains all info of the leaf level field
4652
@Nullable
4753
public abstract Field field();
@@ -70,6 +76,8 @@ public abstract static class Builder {
7076

7177
public abstract HttpBindings.HttpBinding.Builder setName(String name);
7278

79+
public abstract HttpBindings.HttpBinding.Builder setJsonName(String jsonName);
80+
7381
public abstract HttpBindings.HttpBinding.Builder setField(Field field);
7482

7583
abstract HttpBindings.HttpBinding.Builder setLowerCamelName(String lowerCamelName);
@@ -133,9 +141,11 @@ public List<String> lowerCamelAdditionalPatterns() {
133141
private static String lowerCamelPattern(String originalPattern, Set<HttpBinding> pathParameters) {
134142
String lowerCamelPattern = originalPattern;
135143
for (HttpBinding pathParam : pathParameters) {
136-
lowerCamelPattern =
137-
lowerCamelPattern.replaceAll(
138-
"\\{" + pathParam.name(), "{" + JavaStyle.toLowerCamelCase(pathParam.name()));
144+
String replacement =
145+
!Strings.isNullOrEmpty(pathParam.jsonName())
146+
? pathParam.jsonName()
147+
: JavaStyle.toLowerCamelCase(pathParam.name());
148+
lowerCamelPattern = lowerCamelPattern.replaceAll("\\{" + pathParam.name(), "{" + replacement);
139149
}
140150
return lowerCamelPattern;
141151
}

sdk-platform-java/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.google.api.generator.gapic.model.HttpBindings;
2222
import com.google.api.generator.gapic.model.HttpBindings.HttpBinding;
2323
import com.google.api.generator.gapic.model.Message;
24+
import com.google.api.generator.gapic.utils.JavaStyle;
2425
import com.google.api.pathtemplate.PathTemplate;
2526
import com.google.common.base.Preconditions;
2627
import com.google.common.base.Strings;
@@ -29,8 +30,10 @@
2930
import com.google.common.collect.Sets;
3031
import com.google.protobuf.DescriptorProtos.MethodOptions;
3132
import com.google.protobuf.Descriptors.MethodDescriptor;
33+
import java.util.ArrayList;
3234
import java.util.Collections;
3335
import java.util.HashMap;
36+
import java.util.List;
3437
import java.util.Map;
3538
import java.util.Optional;
3639
import java.util.Set;
@@ -144,10 +147,24 @@ private static Set<HttpBinding> validateAndConstructHttpBindings(
144147
continue;
145148
}
146149
Message nestedMessage = inputMessage;
150+
List<String> jsonNameParts = new ArrayList<>();
147151
for (int i = 0; i < subFields.length; i++) {
148152
String subFieldName = subFields[i];
153+
Field field = nestedMessage.fieldMap().get(subFieldName);
154+
Preconditions.checkState(
155+
field != null,
156+
"Expected message %s to contain field %s but none found",
157+
nestedMessage.name(),
158+
subFieldName);
159+
160+
// Each component of the JSON name uses the json_name annotation of the field,
161+
// or default to the field name
162+
jsonNameParts.add(
163+
!Strings.isNullOrEmpty(field.jsonName())
164+
? field.jsonName()
165+
: JavaStyle.toLowerCamelCase(field.name()));
166+
149167
if (i < subFields.length - 1) {
150-
Field field = nestedMessage.fieldMap().get(subFieldName);
151168
nestedMessage = messageTypes.get(field.type().reference().fullName());
152169
Preconditions.checkNotNull(
153170
nestedMessage,
@@ -160,9 +177,12 @@ private static Set<HttpBinding> validateAndConstructHttpBindings(
160177
checkHttpFieldIsValid(subFieldName, nestedMessage, false);
161178
patternSampleValue = patternSampleValues.get(paramName);
162179
}
163-
Field field = nestedMessage.fieldMap().get(subFieldName);
164180
httpBindings.add(
165-
httpBindingBuilder.setValuePattern(patternSampleValue).setField(field).build());
181+
httpBindingBuilder
182+
.setValuePattern(patternSampleValue)
183+
.setField(field)
184+
.setJsonName(String.join(".", jsonNameParts))
185+
.build());
166186
}
167187
}
168188
}

sdk-platform-java/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1181,6 +1181,7 @@ private static Field parseField(
11811181
return fieldBuilder
11821182
.setName(actualFieldName)
11831183
.setOriginalName(fieldDescriptor.getName())
1184+
.setJsonName(fieldDescriptor.getJsonName())
11841185
.setType(TypeParser.parseType(fieldDescriptor))
11851186
.setIsMessage(fieldDescriptor.getJavaType() == FieldDescriptor.JavaType.MESSAGE)
11861187
.setIsEnum(fieldDescriptor.getJavaType() == FieldDescriptor.JavaType.ENUM)

sdk-platform-java/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpcrest/goldens/HttpJsonEchoStub.golden

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ public class HttpJsonEchoStub extends EchoStub {
362362
})
363363
.setRequestBodyExtractor(
364364
request ->
365-
ProtoRestSerializer.create().toBody("case_", request.getCase(), false))
365+
ProtoRestSerializer.create().toBody("case", request.getCase(), false))
366366
.build())
367367
.setResponseParser(
368368
ProtoMessageResponseParser.<Case>newBuilder()

0 commit comments

Comments
 (0)