Skip to content

fix(generator): use json_name when available #12940

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

Open
diegomarquezp wants to merge 7 commits into
base: main
Choose a base branch
from
Open

fix(generator): use json_name when available #12940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
81dd15f
fix(generator): use json_name when available
diegomarquezp Apr 27, 2026
78c5a45
fix: remove unnecessary null check
diegomarquezp Apr 27, 2026
4ec4743
chore: cleanup
diegomarquezp Apr 27, 2026
821cec4
chore: format
diegomarquezp Apr 27, 2026
0967e2c
test: retore missing test
diegomarquezp Apr 27, 2026
2c7cd12
fix: use jupiter assertion
diegomarquezp Apr 27, 2026
aa4005b
fix: use fallback in stub composer
diegomarquezp Apr 27, 2026
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
4 changes: 3 additions & 1 deletion ...n/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceStubClassComposer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,7 @@
.build();
}

private Expr createFieldsExtractorClassInstance(

Check failure on line 830 in sdk-platform-java/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceStubClassComposer.java

View check run for this annotation

SonarQubeCloud / [gapic-generator-java-root] SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 16 to the 15 allowed. 


See more on https://sonarcloud.io/project/issues?id=googleapis_google-cloud-java_generator&issues=AZ3QFk4Qff0uYgqTznWy&open=AZ3QFk4Qff0uYgqTznWy&pullRequest=12940
Method method,
TypeNode extractorReturnType,
Set<HttpBinding> httpBindingFieldNames,
Expand Down Expand Up @@ -926,7 +926,9 @@
paramsPutArgs.add(
ValueExpr.withValue(
StringObjectValue.withValue(
JavaStyle.toLowerCamelCase(httpBindingFieldName.name()))));
(httpBindingFieldName.jsonName() != null)
Copy link
Copy Markdown
Contributor Author

@diegomarquezp diegomarquezp Apr 29, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Let's consider path params, body params and others. Query params is the main scope but others must be considered too.

? httpBindingFieldName.jsonName()
: JavaStyle.toLowerCamelCase(httpBindingFieldName.name()))));
paramsPutArgs.add(requestBuilderExpr);

Expr paramsPutExpr =
Expand Down
7 changes: 7 additions & 0 deletions ...m-java/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ public abstract class Field {
// resolution behavior. For more context, please see the invocation site of the setter method.
public abstract String originalName();

@Nullable
public abstract String jsonName();

public abstract TypeNode type();

// If the field is annotated with google.api.field_behavior = REQUIRED, then this is true. This is
Expand Down Expand Up @@ -92,6 +95,7 @@ public boolean equals(Object o) {
Field other = (Field) o;
return name().equals(other.name())
&& originalName().equals(other.originalName())
&& Objects.equals(jsonName(), other.jsonName())
&& type().equals(other.type())
&& isRequired() == other.isRequired()
&& fieldInfoFormat() == other.fieldInfoFormat()
Expand All @@ -109,6 +113,7 @@ && isProto3Optional() == other.isProto3Optional()
public int hashCode() {
return 17 * name().hashCode()
+ 31 * originalName().hashCode()
+ (jsonName() == null ? 0 : jsonName().hashCode())
+ 19 * type().hashCode()
+ (isMessage() ? 1 : 0) * 23
+ (isEnum() ? 1 : 0) * 29
Expand Down Expand Up @@ -141,6 +146,8 @@ public abstract static class Builder {

public abstract Builder setOriginalName(String originalName);

public abstract Builder setJsonName(String jsonName);

public abstract Builder setType(TypeNode type);

public abstract Builder setIsRequired(boolean isRequired);
Expand Down
7 changes: 7 additions & 0 deletions ...gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/HttpBindings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ public abstract static class HttpBinding implements Comparable<HttpBinding> {

abstract String lowerCamelName();

// The dot-separated json_name of the field.
// e.g. parent.iceberg-catalog-id
@Nullable
public abstract String jsonName();

// An object that contains all info of the leaf level field
@Nullable
public abstract Field field();
Expand Down Expand Up @@ -70,6 +75,8 @@ public abstract static class Builder {

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

public abstract HttpBindings.HttpBinding.Builder setJsonName(String jsonName);

public abstract HttpBindings.HttpBinding.Builder setField(Field field);

abstract HttpBindings.HttpBinding.Builder setLowerCamelName(String lowerCamelName);
Expand Down
26 changes: 23 additions & 3 deletions ...nerator-java/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.api.generator.gapic.model.HttpBindings;
import com.google.api.generator.gapic.model.HttpBindings.HttpBinding;
import com.google.api.generator.gapic.model.Message;
import com.google.api.generator.gapic.utils.JavaStyle;
import com.google.api.pathtemplate.PathTemplate;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
Expand All @@ -29,8 +30,10 @@
import com.google.common.collect.Sets;
import com.google.protobuf.DescriptorProtos.MethodOptions;
import com.google.protobuf.Descriptors.MethodDescriptor;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
Expand Down Expand Up @@ -126,7 +129,7 @@
.build();
}

private static Set<HttpBinding> validateAndConstructHttpBindings(

Check failure on line 132 in sdk-platform-java/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java

View check run for this annotation

SonarQubeCloud / [gapic-generator-java-root] SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 18 to the 15 allowed. 


See more on https://sonarcloud.io/project/issues?id=googleapis_google-cloud-java_generator&issues=AZ3P2XzoMVN7j-fsl9qM&open=AZ3P2XzoMVN7j-fsl9qM&pullRequest=12940
Set<String> paramNames,
Message inputMessage,
Map<String, Message> messageTypes,
Expand All @@ -144,10 +147,24 @@
continue;
}
Message nestedMessage = inputMessage;
List<String> jsonNameParts = new ArrayList<>();
for (int i = 0; i < subFields.length; i++) {
String subFieldName = subFields[i];
Field field = nestedMessage.fieldMap().get(subFieldName);
Preconditions.checkState(
field != null,
"Expected message %s to contain field %s but none found",
nestedMessage.name(),

Check warning on line 157 in sdk-platform-java/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java

View check run for this annotation

SonarQubeCloud / [gapic-generator-java-root] SonarCloud Code Analysis

Invoke method(s) only conditionally. 


See more on https://sonarcloud.io/project/issues?id=googleapis_google-cloud-java_generator&issues=AZ3P4UEL91BcPujooV-Q&open=AZ3P4UEL91BcPujooV-Q&pullRequest=12940
subFieldName);

// Each component of the JSON name uses the json_name annotation of the field,
// or default to the field name
jsonNameParts.add(
!Strings.isNullOrEmpty(field.jsonName())
? field.jsonName()
: JavaStyle.toLowerCamelCase(field.name()));

if (i < subFields.length - 1) {
Field field = nestedMessage.fieldMap().get(subFieldName);
nestedMessage = messageTypes.get(field.type().reference().fullName());
Preconditions.checkNotNull(
nestedMessage,
Expand All @@ -160,9 +177,12 @@
checkHttpFieldIsValid(subFieldName, nestedMessage, false);
patternSampleValue = patternSampleValues.get(paramName);
}
Field field = nestedMessage.fieldMap().get(subFieldName);
httpBindings.add(
httpBindingBuilder.setValuePattern(patternSampleValue).setField(field).build());
httpBindingBuilder
.setValuePattern(patternSampleValue)
.setField(field)
.setJsonName(String.join(".", jsonNameParts))
.build());
}
}
}
Expand Down
1 change: 1 addition & 0 deletions ...gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1181,6 +1181,7 @@ private static Field parseField(
return fieldBuilder
.setName(actualFieldName)
.setOriginalName(fieldDescriptor.getName())
.setJsonName(fieldDescriptor.getJsonName())
.setType(TypeParser.parseType(fieldDescriptor))
.setIsMessage(fieldDescriptor.getJavaType() == FieldDescriptor.JavaType.MESSAGE)
.setIsEnum(fieldDescriptor.getJavaType() == FieldDescriptor.JavaType.ENUM)
Expand Down
12 changes: 12 additions & 0 deletions ...est/java/com/google/api/generator/gapic/composer/rest/goldens/ComplianceClientTest.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public class ComplianceClientTest {
.setName("name3373707")
.setInfo(ComplianceData.newBuilder().build())
.setServerVerify(true)
.setCustomKebabName("customKebabName-2062111197")
.build();

RepeatResponse actualResponse = client.repeatDataBody(request);
Expand Down Expand Up @@ -99,6 +100,7 @@ public class ComplianceClientTest {
.setName("name3373707")
.setInfo(ComplianceData.newBuilder().build())
.setServerVerify(true)
.setCustomKebabName("customKebabName-2062111197")
.build();
client.repeatDataBody(request);
Assert.fail("No exception raised");
Expand All @@ -118,6 +120,7 @@ public class ComplianceClientTest {
.setName("name3373707")
.setInfo(ComplianceData.newBuilder().build())
.setServerVerify(true)
.setCustomKebabName("customKebabName-2062111197")
.build();

RepeatResponse actualResponse = client.repeatDataBodyInfo(request);
Expand Down Expand Up @@ -151,6 +154,7 @@ public class ComplianceClientTest {
.setName("name3373707")
.setInfo(ComplianceData.newBuilder().build())
.setServerVerify(true)
.setCustomKebabName("customKebabName-2062111197")
.build();
client.repeatDataBodyInfo(request);
Assert.fail("No exception raised");
Expand All @@ -170,6 +174,7 @@ public class ComplianceClientTest {
.setName("name3373707")
.setInfo(ComplianceData.newBuilder().build())
.setServerVerify(true)
.setCustomKebabName("customKebabName-2062111197")
.build();

RepeatResponse actualResponse = client.repeatDataQuery(request);
Expand Down Expand Up @@ -203,6 +208,7 @@ public class ComplianceClientTest {
.setName("name3373707")
.setInfo(ComplianceData.newBuilder().build())
.setServerVerify(true)
.setCustomKebabName("customKebabName-2062111197")
.build();
client.repeatDataQuery(request);
Assert.fail("No exception raised");
Expand Down Expand Up @@ -245,6 +251,7 @@ public class ComplianceClientTest {
.setPChild(ComplianceDataChild.newBuilder().build())
.build())
.setServerVerify(true)
.setCustomKebabName("customKebabName-2062111197")
.build();

RepeatResponse actualResponse = client.repeatDataSimplePath(request);
Expand Down Expand Up @@ -301,6 +308,7 @@ public class ComplianceClientTest {
.setPChild(ComplianceDataChild.newBuilder().build())
.build())
.setServerVerify(true)
.setCustomKebabName("customKebabName-2062111197")
.build();
client.repeatDataSimplePath(request);
Assert.fail("No exception raised");
Expand Down Expand Up @@ -357,6 +365,7 @@ public class ComplianceClientTest {
.setPChild(ComplianceDataChild.newBuilder().build())
.build())
.setServerVerify(true)
.setCustomKebabName("customKebabName-2062111197")
.build();

RepeatResponse actualResponse = client.repeatDataPathResource(request);
Expand Down Expand Up @@ -427,6 +436,7 @@ public class ComplianceClientTest {
.setPChild(ComplianceDataChild.newBuilder().build())
.build())
.setServerVerify(true)
.setCustomKebabName("customKebabName-2062111197")
.build();
client.repeatDataPathResource(request);
Assert.fail("No exception raised");
Expand Down Expand Up @@ -483,6 +493,7 @@ public class ComplianceClientTest {
.setPChild(ComplianceDataChild.newBuilder().build())
.build())
.setServerVerify(true)
.setCustomKebabName("customKebabName-2062111197")
.build();

RepeatResponse actualResponse = client.repeatDataPathTrailingResource(request);
Expand Down Expand Up @@ -553,6 +564,7 @@ public class ComplianceClientTest {
.setPChild(ComplianceDataChild.newBuilder().build())
.build())
.setServerVerify(true)
.setCustomKebabName("customKebabName-2062111197")
.build();
client.repeatDataPathTrailingResource(request);
Assert.fail("No exception raised");
Expand Down
10 changes: 10 additions & 0 deletions ...t/java/com/google/api/generator/gapic/composer/rest/goldens/HttpJsonComplianceStub.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ public class HttpJsonComplianceStub extends ComplianceStub {
Map<String, List<String>> fields = new HashMap<>();
ProtoRestSerializer<RepeatRequest> serializer =
ProtoRestSerializer.create();
serializer.putQueryParam(
fields, "custom-kebab-name", request.getCustomKebabName());
serializer.putQueryParam(fields, "name", request.getName());
serializer.putQueryParam(
fields, "serverVerify", request.getServerVerify());
Expand Down Expand Up @@ -132,6 +134,8 @@ public class HttpJsonComplianceStub extends ComplianceStub {
Map<String, List<String>> fields = new HashMap<>();
ProtoRestSerializer<RepeatRequest> serializer =
ProtoRestSerializer.create();
serializer.putQueryParam(
Copy link
Copy Markdown
Contributor Author

@diegomarquezp diegomarquezp Apr 29, 2026

Choose a reason for hiding this comment

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

let's test with biglake directly and confirm that the response conforms to json_name

fields, "custom-kebab-name", request.getCustomKebabName());
serializer.putQueryParam(fields, "info", request.getInfo());
serializer.putQueryParam(fields, "name", request.getName());
serializer.putQueryParam(
Expand Down Expand Up @@ -181,6 +185,8 @@ public class HttpJsonComplianceStub extends ComplianceStub {
Map<String, List<String>> fields = new HashMap<>();
ProtoRestSerializer<RepeatRequest> serializer =
ProtoRestSerializer.create();
serializer.putQueryParam(
fields, "custom-kebab-name", request.getCustomKebabName());
serializer.putQueryParam(fields, "info", request.getInfo());
serializer.putQueryParam(fields, "name", request.getName());
serializer.putQueryParam(
Expand Down Expand Up @@ -228,6 +234,8 @@ public class HttpJsonComplianceStub extends ComplianceStub {
Map<String, List<String>> fields = new HashMap<>();
ProtoRestSerializer<RepeatRequest> serializer =
ProtoRestSerializer.create();
serializer.putQueryParam(
fields, "custom-kebab-name", request.getCustomKebabName());
serializer.putQueryParam(fields, "info", request.getInfo());
serializer.putQueryParam(fields, "name", request.getName());
serializer.putQueryParam(
Expand Down Expand Up @@ -272,6 +280,8 @@ public class HttpJsonComplianceStub extends ComplianceStub {
Map<String, List<String>> fields = new HashMap<>();
ProtoRestSerializer<RepeatRequest> serializer =
ProtoRestSerializer.create();
serializer.putQueryParam(
fields, "custom-kebab-name", request.getCustomKebabName());
serializer.putQueryParam(fields, "info", request.getInfo());
serializer.putQueryParam(fields, "name", request.getName());
serializer.putQueryParam(
Expand Down
8 changes: 8 additions & 0 deletions ...c-generator-java/src/test/java/com/google/api/generator/gapic/model/HttpBindingsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,12 @@ void isEnum_shouldReturnTrueIfFieldExistsAndIsEnumIsTue() {

Truth.assertThat(httpBinding.isEnum()).isTrue();
}

@Test
void builder_preservesLiteralJsonName() {
final String jsonName = "iceberg-catalog-id";
HttpBinding binding =
HttpBinding.builder().setName("doesNotMatter").setJsonName(jsonName).build();
Truth.assertThat(binding.jsonName()).isEqualTo(jsonName);
}
}
35 changes: 34 additions & 1 deletion ...tor-java/src/test/java/com/google/api/generator/gapic/protoparser/HttpRuleParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,15 @@ void parseHttpAnnotation_shouldPutAllFieldsIntoQueryParamsIfPathParamAndBodyAreN
HttpBindings actual = HttpRuleParser.parse(rpcMethod, inputMessage, messages);

HttpBinding expected1 =
HttpBinding.builder().setName("name").setField(inputMessage.fieldMap().get("name")).build();
HttpBinding.builder()
.setName("name")
.setJsonName("name")
.setField(inputMessage.fieldMap().get("name"))
.build();
HttpBinding expected2 =
HttpBinding.builder()
.setName("nested_object")
.setJsonName("nestedObject")
.setField(inputMessage.fieldMap().get("nested_object"))
.build();
Truth.assertThat(new HashSet<>(actual.queryParameters())).containsExactly(expected1, expected2);
Expand Down Expand Up @@ -173,4 +178,32 @@ void parseHttpAnnotation_shouldExcludeFieldsFromQueryParamsIfPathParamsAreConfig
Truth.assertThat(new HashSet<>(actual.queryParameters())).containsExactly(expected1, expected2);
Truth.assertThat(new HashSet<>(actual.pathParameters())).containsExactly(expectedPathParam);
}

@Test
void parseHttpAnnotation_respectsJsonNameWithDashes() {
FileDescriptor complianceFileDescriptor =
com.google.showcase.v1beta1.ComplianceOuterClass.getDescriptor();
ServiceDescriptor complianceService = complianceFileDescriptor.getServices().get(0);
assertEquals("Compliance", complianceService.getName());

Map<String, Message> messages = Parser.parseMessages(complianceFileDescriptor);

MethodDescriptor rpcMethod =
complianceService.getMethods().stream()
.filter(m -> m.getName().equals("RepeatDataQuery"))
.findAny()
.get();

Message inputMessage = messages.get("com.google.showcase.v1beta1.RepeatRequest");
HttpBindings httpBindings = HttpRuleParser.parse(rpcMethod, inputMessage, messages);

HttpBinding customBinding =
httpBindings.queryParameters().stream()
.filter(b -> b.name().equals("custom_kebab_name"))
.findAny()
.orElse(null);

Truth.assertThat(customBinding).isNotNull();
assertEquals("custom-kebab-name", customBinding.jsonName());
}
}
3 changes: 2 additions & 1 deletion ...c-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,11 @@ void parseMessages_basic() {

String echoResponseName = "EchoResponse";
Field echoResponseContentField =
Field.builder().setName("content").setType(TypeNode.STRING).build();
Field.builder().setName("content").setJsonName("content").setType(TypeNode.STRING).build();
Field echoResponseSeverityField =
Field.builder()
.setName("severity")
.setJsonName("severity")
.setType(
TypeNode.withReference(
VaporReference.builder().setName("Severity").setPakkage(ECHO_PACKAGE).build()))
Expand Down
3 changes: 3 additions & 0 deletions sdk-platform-java/gapic-generator-java/src/test/proto/compliance.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ message RepeatRequest {
// If true, the server will verify that the received request matches
// the request with the same name in the compliance test suite.
bool server_verify = 3;

// New field for testing json_name with dashes
string custom_kebab_name = 4 [json_name = "custom-kebab-name"];
Copy link
Copy Markdown
Contributor Author

@diegomarquezp diegomarquezp Apr 29, 2026

Choose a reason for hiding this comment

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

let's try a few more cases e.g. path params and confirm

}

message RepeatResponse {
Expand Down
Loading