From dc3a66b947b67ae83470d4d4dfc481b78196b8b8 Mon Sep 17 00:00:00 2001 From: vam-google Date: Fri, 18 Jun 2021 02:54:57 -0700 Subject: [PATCH 1/2] feat: Implement field presence support for DIREGAPIC --- .../grpc/GrpcServiceStubClassComposer.java | 7 +- .../HttpJsonServiceStubClassComposer.java | 45 +++++++-- .../api/generator/gapic/model/Field.java | 9 +- .../generator/gapic/model/HttpBindings.java | 32 ++++-- .../gapic/protoparser/HttpRuleParser.java | 97 +++++++++++-------- .../generator/gapic/protoparser/Parser.java | 3 + .../goldens/HttpJsonComplianceStub.golden | 12 ++- .../gapic/protoparser/HttpRuleParserTest.java | 13 ++- .../generator/gapic/testdata/compliance.proto | 2 +- .../v1/stub/HttpJsonAddressesStub.java | 45 ++++++--- 10 files changed, 185 insertions(+), 80 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceStubClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceStubClassComposer.java index f1fbbf667f..07ffa34d3b 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceStubClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceStubClassComposer.java @@ -34,6 +34,7 @@ import com.google.api.generator.engine.ast.VariableExpr; import com.google.api.generator.gapic.composer.common.AbstractServiceStubClassComposer; import com.google.api.generator.gapic.composer.store.TypeStore; +import com.google.api.generator.gapic.model.HttpBindings.HttpBinding; import com.google.api.generator.gapic.model.Method; import com.google.api.generator.gapic.model.Service; import com.google.api.generator.gapic.utils.JavaStyle; @@ -288,11 +289,11 @@ private AnonymousClassExpr createRequestParamsExtractorAnonClass(Method method) VariableExpr.withVariable( Variable.builder().setType(method.inputType()).setName("request").build()); - for (String httpBindingFieldName : method.httpBindings().pathParameters()) { + for (HttpBinding httpBindingFieldBinding : method.httpBindings().pathParameters()) { // Handle foo.bar cases by descending into the subfields. MethodInvocationExpr.Builder requestFieldGetterExprBuilder = MethodInvocationExpr.builder().setExprReferenceExpr(requestVarExpr); - String[] descendantFields = httpBindingFieldName.split("\\."); + String[] descendantFields = httpBindingFieldBinding.name().split("\\."); for (int i = 0; i < descendantFields.length; i++) { String currFieldName = descendantFields[i]; String bindingFieldMethodName = @@ -319,7 +320,7 @@ private AnonymousClassExpr createRequestParamsExtractorAnonClass(Method method) .setExprReferenceExpr(paramsVarExpr) .setMethodName("put") .setArguments( - ValueExpr.withValue(StringObjectValue.withValue(httpBindingFieldName)), + ValueExpr.withValue(StringObjectValue.withValue(httpBindingFieldBinding.name())), valueOfExpr) .build(); bodyExprs.add(paramsPutExpr); diff --git a/src/main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceStubClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceStubClassComposer.java index 093ccdbab3..1dbae8e302 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceStubClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceStubClassComposer.java @@ -30,6 +30,7 @@ import com.google.api.generator.engine.ast.EnumRefExpr; import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; +import com.google.api.generator.engine.ast.IfStatement; import com.google.api.generator.engine.ast.MethodDefinition; import com.google.api.generator.engine.ast.MethodInvocationExpr; import com.google.api.generator.engine.ast.NewObjectExpr; @@ -42,6 +43,7 @@ import com.google.api.generator.engine.ast.VariableExpr; import com.google.api.generator.gapic.composer.common.AbstractServiceStubClassComposer; import com.google.api.generator.gapic.composer.store.TypeStore; +import com.google.api.generator.gapic.model.HttpBindings.HttpBinding; import com.google.api.generator.gapic.model.Method; import com.google.api.generator.gapic.model.Service; import com.google.api.generator.gapic.utils.JavaStyle; @@ -357,9 +359,9 @@ private List setResponseParserExpr(Method protoMethod) { private Expr createFieldsExtractorAnonClass( Method method, TypeNode extractorReturnType, - Set httpBindingFieldNames, + Set httpBindingFieldNames, String serializerMethodName) { - List bodyExprs = new ArrayList<>(); + List bodyStatements = new ArrayList<>(); Expr returnExpr = null; VariableExpr fieldsVarExpr = null; @@ -389,7 +391,7 @@ private Expr createFieldsExtractorAnonClass( .build()) .build(); - bodyExprs.add(fieldsAssignExpr); + bodyStatements.add(ExprStatement.withExpr(fieldsAssignExpr)); returnExpr = fieldsVarExpr; TypeNode serializerVarType = @@ -417,32 +419,48 @@ private Expr createFieldsExtractorAnonClass( serializerExpr = serializerVarExpr; - bodyExprs.add(serializerAssignExpr); + bodyStatements.add(ExprStatement.withExpr(serializerAssignExpr)); } VariableExpr requestVarExpr = VariableExpr.withVariable( Variable.builder().setType(method.inputType()).setName("request").build()); - for (String httpBindingFieldName : httpBindingFieldNames) { + for (HttpBinding httpBindingFieldName : httpBindingFieldNames) { // Handle foo.bar cases by descending into the subfields. MethodInvocationExpr.Builder requestFieldGetterExprBuilder = MethodInvocationExpr.builder().setExprReferenceExpr(requestVarExpr); - String[] descendantFields = httpBindingFieldName.split("\\."); + MethodInvocationExpr.Builder requestFieldHasExprBuilder = + MethodInvocationExpr.builder().setExprReferenceExpr(requestVarExpr); + String[] descendantFields = httpBindingFieldName.name().split("\\."); for (int i = 0; i < descendantFields.length; i++) { String currFieldName = descendantFields[i]; String bindingFieldMethodName = String.format("get%s", JavaStyle.toUpperCamelCase(currFieldName)); requestFieldGetterExprBuilder = requestFieldGetterExprBuilder.setMethodName(bindingFieldMethodName); + + String bindingFieldHasMethodName = + (i < descendantFields.length - 1) + ? bindingFieldMethodName + : String.format("has%s", JavaStyle.toUpperCamelCase(currFieldName)); + requestFieldHasExprBuilder = + requestFieldHasExprBuilder + .setMethodName(bindingFieldHasMethodName) + .setReturnType(TypeNode.BOOLEAN); + if (i < descendantFields.length - 1) { requestFieldGetterExprBuilder = MethodInvocationExpr.builder() .setExprReferenceExpr(requestFieldGetterExprBuilder.build()); + requestFieldHasExprBuilder = + MethodInvocationExpr.builder() + .setExprReferenceExpr(requestFieldHasExprBuilder.build()); } } MethodInvocationExpr requestBuilderExpr = requestFieldGetterExprBuilder.build(); + MethodInvocationExpr requestHasExpr = requestFieldHasExprBuilder.build(); ImmutableList.Builder paramsPutArgs = ImmutableList.builder(); if (fieldsVarExpr != null) { @@ -450,7 +468,8 @@ private Expr createFieldsExtractorAnonClass( } paramsPutArgs.add( ValueExpr.withValue( - StringObjectValue.withValue(JavaStyle.toLowerCamelCase(httpBindingFieldName)))); + StringObjectValue.withValue( + JavaStyle.toLowerCamelCase(httpBindingFieldName.name())))); paramsPutArgs.add(requestBuilderExpr); Expr paramsPutExpr = @@ -464,7 +483,15 @@ private Expr createFieldsExtractorAnonClass( if (fieldsVarExpr == null) { returnExpr = paramsPutExpr; } else { - bodyExprs.add(paramsPutExpr); + if (httpBindingFieldName.isOptional()) { + bodyStatements.add( + IfStatement.builder() + .setConditionExpr(requestHasExpr) + .setBody(Arrays.asList(ExprStatement.withExpr(paramsPutExpr))) + .build()); + } else { + bodyStatements.add(ExprStatement.withExpr(paramsPutExpr)); + } } } @@ -475,7 +502,7 @@ private Expr createFieldsExtractorAnonClass( .setReturnType(extractorReturnType) .setName("extract") .setArguments(requestVarExpr.toBuilder().setIsDecl(true).build()) - .setBody(bodyExprs.stream().map(ExprStatement::withExpr).collect(Collectors.toList())) + .setBody(bodyStatements) .setReturnExpr(returnExpr) .build(); diff --git a/src/main/java/com/google/api/generator/gapic/model/Field.java b/src/main/java/com/google/api/generator/gapic/model/Field.java index e178e417e6..555af4f435 100644 --- a/src/main/java/com/google/api/generator/gapic/model/Field.java +++ b/src/main/java/com/google/api/generator/gapic/model/Field.java @@ -35,6 +35,8 @@ public abstract class Field { public abstract boolean isContainedInOneof(); + public abstract boolean isProto3Optional(); + @Nullable public abstract ResourceReference resourceReference(); @@ -63,6 +65,7 @@ && isEnum() == other.isEnum() && isRepeated() == other.isRepeated() && isMap() == other.isMap() && isContainedInOneof() == other.isContainedInOneof() + && isProto3Optional() == other.isProto3Optional() && Objects.equals(resourceReference(), other.resourceReference()) && Objects.equals(description(), other.description()); } @@ -76,6 +79,7 @@ public int hashCode() { + (isRepeated() ? 1 : 0) * 31 + (isMap() ? 1 : 0) * 37 + (isContainedInOneof() ? 1 : 0) * 41 + + (isProto3Optional() ? 1 : 0) * 43 + (resourceReference() == null ? 0 : resourceReference().hashCode()) + (description() == null ? 0 : description().hashCode()); } @@ -88,7 +92,8 @@ public static Builder builder() { .setIsEnum(false) .setIsRepeated(false) .setIsMap(false) - .setIsContainedInOneof(false); + .setIsContainedInOneof(false) + .setIsProto3Optional(false); } @AutoValue.Builder @@ -107,6 +112,8 @@ public abstract static class Builder { public abstract Builder setIsContainedInOneof(boolean isContainedInOneof); + public abstract Builder setIsProto3Optional(boolean isProto3Optional); + public abstract Builder setResourceReference(ResourceReference resourceReference); public abstract Builder setDescription(String description); diff --git a/src/main/java/com/google/api/generator/gapic/model/HttpBindings.java b/src/main/java/com/google/api/generator/gapic/model/HttpBindings.java index 198332af63..e37faa69d3 100644 --- a/src/main/java/com/google/api/generator/gapic/model/HttpBindings.java +++ b/src/main/java/com/google/api/generator/gapic/model/HttpBindings.java @@ -29,15 +29,31 @@ public enum HttpVerb { PATCH, } + @AutoValue + public abstract static class HttpBinding implements Comparable { + public abstract String name(); + + public abstract boolean isOptional(); + + public static HttpBinding create(String name, boolean isOptional) { + return new AutoValue_HttpBindings_HttpBinding(name, isOptional); + } + + @Override + public int compareTo(HttpBinding o) { + return name().compareTo(o.name()); + } + } + public abstract HttpVerb httpVerb(); public abstract String pattern(); - public abstract Set pathParameters(); + public abstract Set pathParameters(); - public abstract Set queryParameters(); + public abstract Set queryParameters(); - public abstract Set bodyParameters(); + public abstract Set bodyParameters(); public static HttpBindings.Builder builder() { return new AutoValue_HttpBindings.Builder() @@ -53,10 +69,10 @@ public static HttpBindings.Builder builder() { // in .java file: "/global/instanceTemplates/{instanceTemplate=*}" public String patternLowerCamel() { String lowerCamelPattern = pattern(); - for (String pathParam : pathParameters()) { + for (HttpBinding pathParam : pathParameters()) { lowerCamelPattern = lowerCamelPattern.replaceAll( - "\\{" + pathParam, "{" + JavaStyle.toLowerCamelCase(pathParam)); + "\\{" + pathParam.name(), "{" + JavaStyle.toLowerCamelCase(pathParam.name())); } return lowerCamelPattern; } @@ -69,11 +85,11 @@ public abstract static class Builder { abstract String pattern(); - public abstract HttpBindings.Builder setPathParameters(Set pathParameters); + public abstract HttpBindings.Builder setPathParameters(Set pathParameters); - public abstract HttpBindings.Builder setQueryParameters(Set queryParameters); + public abstract HttpBindings.Builder setQueryParameters(Set queryParameters); - public abstract HttpBindings.Builder setBodyParameters(Set bodyParameters); + public abstract HttpBindings.Builder setBodyParameters(Set bodyParameters); public abstract HttpBindings autoBuild(); diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java index 1d2f86245c..26629949ef 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java @@ -19,6 +19,7 @@ import com.google.api.HttpRule.PatternCase; import com.google.api.generator.gapic.model.Field; 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.pathtemplate.PathTemplate; import com.google.common.base.Preconditions; @@ -70,63 +71,79 @@ private static HttpBindings parseHttpRuleHelper( } } - Set bindings = bindingsBuilder.build(); - - // Binding validation. - for (String binding : bindings) { - // Handle foo.bar cases by descending into the subfields. - String[] descendantBindings = binding.split("\\."); - Optional containingMessageOpt = inputMessageOpt; - for (int i = 0; i < descendantBindings.length; i++) { - String subField = descendantBindings[i]; - if (!containingMessageOpt.isPresent()) { - continue; - } - - if (i < descendantBindings.length - 1) { - Field field = containingMessageOpt.get().fieldMap().get(subField); - containingMessageOpt = Optional.of(messageTypes.get(field.type().reference().fullName())); - Preconditions.checkNotNull( - containingMessageOpt.get(), - String.format( - "No containing message found for field %s with type %s", - field.name(), field.type().reference().simpleName())); - } else { - checkHttpFieldIsValid(subField, containingMessageOpt.get(), false); - } - } - } + Set pathParamNames = bindingsBuilder.build(); // TODO: support nested message fields bindings String body = httpRule.getBody(); - Set bodyParameters; - Set queryParameters; + Set bodyParamNames; + Set queryParamNames; if (!inputMessageOpt.isPresent()) { // Must be a mixin, do not support full HttpRuleBindings for now - bodyParameters = ImmutableSet.of(); - queryParameters = ImmutableSet.of(); + bodyParamNames = ImmutableSet.of(); + queryParamNames = ImmutableSet.of(); } else if (Strings.isNullOrEmpty(body)) { - bodyParameters = ImmutableSet.of(); - queryParameters = Sets.difference(inputMessageOpt.get().fieldMap().keySet(), bindings); + bodyParamNames = ImmutableSet.of(); + queryParamNames = Sets.difference(inputMessageOpt.get().fieldMap().keySet(), pathParamNames); } else if (body.equals(ASTERISK)) { - bodyParameters = Sets.difference(inputMessageOpt.get().fieldMap().keySet(), bindings); - queryParameters = ImmutableSet.of(); + bodyParamNames = Sets.difference(inputMessageOpt.get().fieldMap().keySet(), pathParamNames); + queryParamNames = ImmutableSet.of(); } else { - bodyParameters = ImmutableSet.of(body); - Set bodyBinidngsUnion = Sets.union(bodyParameters, bindings); - queryParameters = + bodyParamNames = ImmutableSet.of(body); + Set bodyBinidngsUnion = Sets.union(bodyParamNames, pathParamNames); + queryParamNames = Sets.difference(inputMessageOpt.get().fieldMap().keySet(), bodyBinidngsUnion); } + Message message = inputMessageOpt.orElse(null); return HttpBindings.builder() .setHttpVerb(HttpBindings.HttpVerb.valueOf(httpRule.getPatternCase().toString())) .setPattern(pattern) - .setPathParameters(ImmutableSortedSet.copyOf(bindings)) - .setQueryParameters(ImmutableSortedSet.copyOf(queryParameters)) - .setBodyParameters(ImmutableSortedSet.copyOf(bodyParameters)) + .setPathParameters( + validateAndConstructHttpBindings(pathParamNames, message, messageTypes, true)) + .setQueryParameters( + validateAndConstructHttpBindings(queryParamNames, message, messageTypes, false)) + .setBodyParameters( + validateAndConstructHttpBindings(bodyParamNames, message, messageTypes, false)) .build(); } + private static Set validateAndConstructHttpBindings( + Set paramNames, + Message inputMessage, + Map messageTypes, + boolean isPath) { + ImmutableSortedSet.Builder httpBindings = ImmutableSortedSet.naturalOrder(); + for (String paramName : paramNames) { + // Handle foo.bar cases by descending into the subfields. + String[] subFields = paramName.split("\\."); + if (inputMessage == null) { + httpBindings.add(HttpBinding.create(paramName, false)); + continue; + } + Message nestedMessage = inputMessage; + for (int i = 0; i < subFields.length; i++) { + String subFieldName = subFields[i]; + if (i < subFields.length - 1) { + Field field = nestedMessage.fieldMap().get(subFieldName); + nestedMessage = messageTypes.get(field.type().reference().fullName()); + Preconditions.checkNotNull( + nestedMessage, + String.format( + "No containing message found for field %s with type %s", + field.name(), field.type().reference().simpleName())); + + } else { + if (isPath) { + checkHttpFieldIsValid(subFieldName, nestedMessage, !isPath); + } + Field field = nestedMessage.fieldMap().get(subFieldName); + httpBindings.add(HttpBinding.create(paramName, field.isProto3Optional())); + } + } + } + return httpBindings.build(); + } + private static String getHttpVerbPattern(HttpRule httpRule) { PatternCase patternCase = httpRule.getPatternCase(); switch (patternCase) { diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 051e56f873..8ba197b7d1 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -890,6 +890,9 @@ private static Field parseField( .setIsContainedInOneof( fieldDescriptor.getContainingOneof() != null && !fieldDescriptor.getContainingOneof().isSynthetic()) + .setIsProto3Optional( + fieldDescriptor.getContainingOneof() != null + && fieldDescriptor.getContainingOneof().isSynthetic()) .setIsRepeated(fieldDescriptor.isRepeated()) .setIsMap(fieldDescriptor.isMapField()) .setResourceReference(resourceReference) diff --git a/src/test/java/com/google/api/generator/gapic/composer/rest/goldens/HttpJsonComplianceStub.golden b/src/test/java/com/google/api/generator/gapic/composer/rest/goldens/HttpJsonComplianceStub.golden index 2c95e74abb..1ddcb3bf39 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/rest/goldens/HttpJsonComplianceStub.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/rest/goldens/HttpJsonComplianceStub.golden @@ -183,8 +183,10 @@ public class HttpJsonComplianceStub extends ComplianceStub { Map fields = new HashMap<>(); ProtoRestSerializer serializer = ProtoRestSerializer.create(); - serializer.putPathParam( - fields, "info.fBool", request.getInfo().getFBool()); + if (request.getInfo().hasFBool()) { + serializer.putPathParam( + fields, "info.fBool", request.getInfo().getFBool()); + } serializer.putPathParam( fields, "info.fDouble", request.getInfo().getFDouble()); serializer.putPathParam( @@ -239,8 +241,10 @@ public class HttpJsonComplianceStub extends ComplianceStub { Map fields = new HashMap<>(); ProtoRestSerializer serializer = ProtoRestSerializer.create(); - serializer.putPathParam( - fields, "info.fBool", request.getInfo().getFBool()); + if (request.getInfo().hasFBool()) { + serializer.putPathParam( + fields, "info.fBool", request.getInfo().getFBool()); + } serializer.putPathParam( fields, "info.fChild.fString", diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/HttpRuleParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/HttpRuleParserTest.java index c93731b3d8..468e1e392f 100644 --- a/src/test/java/com/google/api/generator/gapic/protoparser/HttpRuleParserTest.java +++ b/src/test/java/com/google/api/generator/gapic/protoparser/HttpRuleParserTest.java @@ -20,12 +20,14 @@ import static org.junit.Assert.assertThrows; 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.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.MethodDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; import com.google.showcase.v1beta1.TestingOuterClass; import java.util.Map; +import java.util.stream.Collectors; import org.junit.Test; public class HttpRuleParserTest { @@ -47,7 +49,11 @@ public void parseHttpAnnotation_basic() { rpcMethod = testingService.getMethods().get(1); inputMessage = messages.get("com.google.showcase.v1beta1.GetSessionRequest"); httpBindings = HttpRuleParser.parse(rpcMethod, inputMessage, messages); - assertThat(httpBindings.pathParameters()).containsExactly("name"); + assertThat( + httpBindings.pathParameters().stream() + .map(HttpBinding::name) + .collect(Collectors.toList())) + .containsExactly("name"); } @Test @@ -63,7 +69,10 @@ public void parseHttpAnnotation_multipleBindings() { testingService.getMethods().get(testingService.getMethods().size() - 1); Message inputMessage = messages.get("com.google.showcase.v1beta1.VerifyTestRequest"); HttpBindings httpBindings = HttpRuleParser.parse(rpcMethod, inputMessage, messages); - assertThat(httpBindings.pathParameters()) + assertThat( + httpBindings.pathParameters().stream() + .map(HttpBinding::name) + .collect(Collectors.toList())) .containsExactly("answer", "foo", "name", "test_to_verify.name", "type"); } diff --git a/src/test/java/com/google/api/generator/gapic/testdata/compliance.proto b/src/test/java/com/google/api/generator/gapic/testdata/compliance.proto index 43b320644c..c602452b13 100644 --- a/src/test/java/com/google/api/generator/gapic/testdata/compliance.proto +++ b/src/test/java/com/google/api/generator/gapic/testdata/compliance.proto @@ -143,7 +143,7 @@ message ComplianceData { double f_double = 12; float f_float = 13; - bool f_bool = 14; + optional bool f_bool = 14; bytes f_bytes = 15; diff --git a/test/integration/goldens/compute/com/google/cloud/compute/v1/stub/HttpJsonAddressesStub.java b/test/integration/goldens/compute/com/google/cloud/compute/v1/stub/HttpJsonAddressesStub.java index 74f92293c9..53a0eb8425 100644 --- a/test/integration/goldens/compute/com/google/cloud/compute/v1/stub/HttpJsonAddressesStub.java +++ b/test/integration/goldens/compute/com/google/cloud/compute/v1/stub/HttpJsonAddressesStub.java @@ -87,13 +87,24 @@ public Map> extract( Map> fields = new HashMap<>(); ProtoRestSerializer serializer = ProtoRestSerializer.create(); - serializer.putQueryParam(fields, "filter", request.getFilter()); - serializer.putQueryParam( - fields, "includeAllScopes", request.getIncludeAllScopes()); - serializer.putQueryParam( - fields, "maxResults", request.getMaxResults()); - serializer.putQueryParam(fields, "orderBy", request.getOrderBy()); - serializer.putQueryParam(fields, "pageToken", request.getPageToken()); + if (request.hasFilter()) { + serializer.putQueryParam(fields, "filter", request.getFilter()); + } + if (request.hasIncludeAllScopes()) { + serializer.putQueryParam( + fields, "includeAllScopes", request.getIncludeAllScopes()); + } + if (request.hasMaxResults()) { + serializer.putQueryParam( + fields, "maxResults", request.getMaxResults()); + } + if (request.hasOrderBy()) { + serializer.putQueryParam(fields, "orderBy", request.getOrderBy()); + } + if (request.hasPageToken()) { + serializer.putQueryParam( + fields, "pageToken", request.getPageToken()); + } return fields; } }) @@ -138,7 +149,9 @@ public Map> extract(DeleteAddressRequest request) { Map> fields = new HashMap<>(); ProtoRestSerializer serializer = ProtoRestSerializer.create(); - serializer.putQueryParam(fields, "requestId", request.getRequestId()); + if (request.hasRequestId()) { + serializer.putQueryParam(fields, "requestId", request.getRequestId()); + } return fields; } }) @@ -182,7 +195,9 @@ public Map> extract(InsertAddressRequest request) { Map> fields = new HashMap<>(); ProtoRestSerializer serializer = ProtoRestSerializer.create(); - serializer.putQueryParam(fields, "requestId", request.getRequestId()); + if (request.hasRequestId()) { + serializer.putQueryParam(fields, "requestId", request.getRequestId()); + } return fields; } }) @@ -227,10 +242,16 @@ public Map> extract(ListAddressesRequest request) { Map> fields = new HashMap<>(); ProtoRestSerializer serializer = ProtoRestSerializer.create(); - serializer.putQueryParam(fields, "filter", request.getFilter()); - serializer.putQueryParam(fields, "maxResults", request.getMaxResults()); + if (request.hasFilter()) { + serializer.putQueryParam(fields, "filter", request.getFilter()); + } + if (request.hasMaxResults()) { + serializer.putQueryParam(fields, "maxResults", request.getMaxResults()); + } serializer.putQueryParam(fields, "orderBy", request.getOrderBy()); - serializer.putQueryParam(fields, "pageToken", request.getPageToken()); + if (request.hasPageToken()) { + serializer.putQueryParam(fields, "pageToken", request.getPageToken()); + } return fields; } }) From 84b542e125f42ff51fb5c3e1a0724bcd29168247 Mon Sep 17 00:00:00 2001 From: vam-google Date: Fri, 18 Jun 2021 17:09:23 -0700 Subject: [PATCH 2/2] address PR feedback --- .../com/google/api/generator/gapic/model/HttpBindings.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/generator/gapic/model/HttpBindings.java b/src/main/java/com/google/api/generator/gapic/model/HttpBindings.java index e37faa69d3..9ca67cf4c7 100644 --- a/src/main/java/com/google/api/generator/gapic/model/HttpBindings.java +++ b/src/main/java/com/google/api/generator/gapic/model/HttpBindings.java @@ -39,9 +39,11 @@ public static HttpBinding create(String name, boolean isOptional) { return new AutoValue_HttpBindings_HttpBinding(name, isOptional); } + // Do not forget to keep it in sync with equals() implementation. @Override public int compareTo(HttpBinding o) { - return name().compareTo(o.name()); + int res = name().compareTo(o.name()); + return res == 0 ? Boolean.compare(isOptional(), o.isOptional()) : res; } }