From 6a4e14a288fbb4c7d3bbd8071da488180783b053 Mon Sep 17 00:00:00 2001 From: Alice Li Date: Mon, 18 Dec 2023 16:54:19 -0500 Subject: [PATCH 01/14] feat: add parsing of autopopulated fields from serviceyaml --- .../api/generator/gapic/model/Field.java | 14 ++++++ .../api/generator/gapic/model/Method.java | 12 +++++ .../generator/gapic/protoparser/Parser.java | 46 +++++++++++++++++++ .../DefaultValueComposerTest.java | 1 + ...lientCallableMethodSampleComposerTest.java | 9 +++- ...ServiceClientHeaderSampleComposerTest.java | 7 ++- ...ServiceClientMethodSampleComposerTest.java | 2 + .../gapic/protoparser/ParserTest.java | 38 +++++++++++++++ .../protoparser/ServiceYamlParserTest.java | 20 ++++++++ .../src/test/proto/echo.proto | 13 ++++++ .../src/test/resources/echo_v1beta1.yaml | 7 ++- 11 files changed, 166 insertions(+), 3 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java index a5ec9caacb..a1cf0b8cc4 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java @@ -14,6 +14,7 @@ package com.google.api.generator.gapic.model; +import com.google.api.FieldInfo.Format; import com.google.api.generator.engine.ast.TypeNode; import com.google.auto.value.AutoValue; import java.util.Objects; @@ -32,6 +33,13 @@ public abstract class Field { public abstract TypeNode type(); + // If the field contains a google.api.field_info.format value of UUID4 and is not marked as REQUIRED, then it is eligible to be auto-populated if the customer does not set it. + // The field needs to be cross-referenced against Method.autoPopulatedFields() to make the final determination if it *should* be auto-populated. + // See go/client-populate-request-id-design for more details. + public abstract boolean isEligibleToBeAutoPopulated(); + + @Nullable + public abstract Format fieldInfoFormat(); public abstract boolean isMessage(); public abstract boolean isEnum(); @@ -72,6 +80,7 @@ public boolean equals(Object o) { return name().equals(other.name()) && originalName().equals(other.originalName()) && type().equals(other.type()) + && isEligibleToBeAutoPopulated() == other.isEligibleToBeAutoPopulated() && isMessage() == other.isMessage() && isEnum() == other.isEnum() && isRepeated() == other.isRepeated() @@ -101,6 +110,7 @@ public int hashCode() { public static Builder builder() { return new AutoValue_Field.Builder() + .setIsEligibleToBeAutoPopulated(false) .setIsMessage(false) .setIsEnum(false) .setIsRepeated(false) @@ -117,6 +127,10 @@ public abstract static class Builder { public abstract Builder setType(TypeNode type); + public abstract Builder setIsEligibleToBeAutoPopulated(boolean isEligibleToBeAutoPopulated); + + public abstract Builder setFieldInfoFormat(Format fieldInfoFormat); + public abstract Builder setIsMessage(boolean isMessage); public abstract Builder setIsEnum(boolean isEnum); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Method.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Method.java index 4fd5c4f384..6c94d917e9 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Method.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Method.java @@ -69,6 +69,16 @@ public boolean isPaged() { // [["content", "error"], ["content", "error", "info"]]. public abstract ImmutableList> methodSignatures(); + @Nullable + public abstract List autoPopulatedFields(); + + /** + * If a service's service_config.yaml file contains method_settings.auto_populated_fields for this method, and the method is a Unary-type, then this is true + */ + public boolean hasAutoPopulatedFields(){ + return autoPopulatedFields() != null && autoPopulatedFields().size() > 0 && stream().equals(Stream.NONE); + } + public abstract boolean operationPollingMethod(); public boolean hasLro() { @@ -170,6 +180,8 @@ public abstract static class Builder { public abstract Builder setOperationPollingMethod(boolean operationPollingMethod); + public abstract Builder setAutoPopulatedFields(List autoPopulatedFields); + public abstract Builder setRoutingHeaderRule(RoutingHeaderRule routingHeaderRule); public abstract Method build(); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index c13eda3718..211c5c82f0 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -16,7 +16,11 @@ import com.google.api.ClientProto; import com.google.api.DocumentationRule; +import com.google.api.FieldBehaviorProto; +import com.google.api.FieldInfo.Format; +import com.google.api.FieldInfoProto; import com.google.api.HttpRule; +import com.google.api.MethodSettings; import com.google.api.ResourceDescriptor; import com.google.api.ResourceProto; import com.google.api.generator.engine.ast.TypeNode; @@ -47,6 +51,7 @@ import com.google.common.collect.BiMap; import com.google.common.collect.HashBiMap; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; import com.google.longrunning.OperationInfo; @@ -493,6 +498,7 @@ public static List parseService( messageTypes, resourceNames, serviceConfigOpt, + serviceYamlProtoOpt, outputArgResourceNames, transport)) .build(); @@ -683,9 +689,25 @@ static List parseMethods( Map messageTypes, Map resourceNames, Optional serviceConfigOpt, + Optional serviceYamlProtoOpt, Set outputArgResourceNames, Transport transport) { List methods = new ArrayList<>(); + + // Parse the serviceYaml for autopopulated methods and fields once and put into a map + ImmutableMap.Builder >autoPopulatedMethodsWithFieldsBuilder = ImmutableMap.builder(); + if (serviceYamlProtoOpt.isPresent()) { + if (serviceYamlProtoOpt.get().getPublishing().getMethodSettingsList().size() > 0) { + for (MethodSettings methodSettings : serviceYamlProtoOpt.get().getPublishing() + .getMethodSettingsList()) { + if(methodSettings.getAutoPopulatedFieldsCount() > 0){ + autoPopulatedMethodsWithFieldsBuilder.put(methodSettings.getSelector(), methodSettings.getAutoPopulatedFieldsList()); + } + } + } + } + ImmutableMap> autoPopulatedMethodsWithFields = autoPopulatedMethodsWithFieldsBuilder.build(); + for (MethodDescriptor protoMethod : serviceDescriptor.getMethods()) { // Parse the method. TypeNode inputType = TypeParser.parseType(protoMethod.getInputType()); @@ -699,6 +721,12 @@ static List parseMethods( } } + // Parse the serviceYaml for autopopulated fields + List autoPopulatedFields = null; + if (autoPopulatedMethodsWithFields.containsKey(protoMethod.getFullName())) { + autoPopulatedFields = autoPopulatedMethodsWithFields.get(protoMethod.getFullName()); + } + boolean isDeprecated = false; if (protoMethod.getOptions().hasDeprecated()) { isDeprecated = protoMethod.getOptions().getDeprecated(); @@ -743,6 +771,7 @@ static List parseMethods( resourceNames, outputArgResourceNames)) .setHttpBindings(httpBindings) + .setAutoPopulatedFields(autoPopulatedFields) .setRoutingHeaderRule(routingHeaderRule) .setIsBatching(isBatching) .setPageSizeFieldName(parsePageSizeFieldName(protoMethod, messageTypes, transport)) @@ -970,6 +999,8 @@ private static Field parseField( FieldOptions fieldOptions = fieldDescriptor.getOptions(); MessageOptions messageOptions = messageDescriptor.getOptions(); ResourceReference resourceReference = null; + boolean isEligibleToBeAutoPopulated = false; + Format fieldInfoFormat = null; if (fieldOptions.hasExtension(ResourceProto.resourceReference)) { com.google.api.ResourceReference protoResourceReference = fieldOptions.getExtension(ResourceProto.resourceReference); @@ -1000,6 +1031,19 @@ private static Field parseField( } } + // If field is annotated with fieldInfo.format = UUID4 and fieldBehavior != REQUIRED, then autopopulate the field + // TODO: Autopopulation requires fieldInfo.formatValue = UUID4; when more types are supported, this logic will need to be updated + if (fieldOptions.hasExtension(FieldInfoProto.fieldInfo)) { + fieldInfoFormat = fieldOptions.getExtension(FieldInfoProto.fieldInfo).getFormat(); + if (fieldOptions.getExtension(FieldInfoProto.fieldInfo).getFormatValue() == 1) { + isEligibleToBeAutoPopulated = true; + } + } + if (fieldOptions.getExtensionCount(FieldBehaviorProto.fieldBehavior) > 0){ + if(fieldOptions.getExtension(FieldBehaviorProto.fieldBehavior).contains(2)); + isEligibleToBeAutoPopulated = false; + } + Field.Builder fieldBuilder = Field.builder(); if (fieldDescriptor.getFile().toProto().hasSourceCodeInfo()) { SourceCodeInfoLocation protoFieldLocation = @@ -1030,6 +1074,8 @@ private static Field parseField( fieldDescriptor.getContainingOneof() != null && fieldDescriptor.getContainingOneof().isSynthetic()) .setIsRepeated(fieldDescriptor.isRepeated()) + .setIsEligibleToBeAutoPopulated(isEligibleToBeAutoPopulated) + .setFieldInfoFormat(fieldInfoFormat) .setIsMap(fieldDescriptor.isMapField()) .setResourceReference(resourceReference) .build(); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/defaultvalue/DefaultValueComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/defaultvalue/DefaultValueComposerTest.java index 264060f474..43e5411b30 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/defaultvalue/DefaultValueComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/defaultvalue/DefaultValueComposerTest.java @@ -568,6 +568,7 @@ public void createSimpleMessage_containsMessagesEnumsAndResourceName() { "EchoRequest.newBuilder().setName(" + "FoobarName.ofProjectFoobarName(\"[PROJECT]\", \"[FOOBAR]\").toString())" + ".setParent(FoobarName.ofProjectFoobarName(\"[PROJECT]\", \"[FOOBAR]\").toString())" + + ".setRequestId(\"requestId693933066\")" + ".setSeverity(Severity.forNumber(0))" + ".setFoobar(Foobar.newBuilder().build()).build()", writerVisitor.write()); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientCallableMethodSampleComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientCallableMethodSampleComposerTest.java index f063c50903..44a921f3b0 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientCallableMethodSampleComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientCallableMethodSampleComposerTest.java @@ -462,7 +462,11 @@ public void valid_composeStreamCallableMethod_serverStream() { LineFormatter.lines( "try (EchoClient echoClient = EchoClient.create()) {\n", " ExpandRequest request =\n", - " ExpandRequest.newBuilder().setContent(\"content951530617\").setInfo(\"info3237038\").build();\n", + " ExpandRequest.newBuilder()\n", + " .setContent(\"content951530617\")\n", + " .setInfo(\"info3237038\")\n", + " .setRequestId(\"requestId693933066\")\n", + " .build();\n", " ServerStream stream = echoClient.expandCallable().call(request);\n", " for (EchoResponse response : stream) {\n", " // Do something when a response is received.\n", @@ -575,6 +579,7 @@ public void valid_composeStreamCallableMethod_bidiStream() { + " \"[FOOBAR]\").toString())\n", " .setParent(FoobarName.ofProjectFoobarName(\"[PROJECT]\"," + " \"[FOOBAR]\").toString())\n", + " .setRequestId(\"requestId693933066\")\n", " .setSeverity(Severity.forNumber(0))\n", " .setFoobar(Foobar.newBuilder().build())\n", " .build();\n", @@ -707,6 +712,7 @@ public void valid_composeStreamCallableMethod_clientStream() { + " \"[FOOBAR]\").toString())\n", " .setParent(FoobarName.ofProjectFoobarName(\"[PROJECT]\"," + " \"[FOOBAR]\").toString())\n", + " .setRequestId(\"requestId693933066\")\n", " .setSeverity(Severity.forNumber(0))\n", " .setFoobar(Foobar.newBuilder().build())\n", " .build();\n", @@ -813,6 +819,7 @@ public void valid_composeRegularCallableMethod_unaryRpc() { + " \"[FOOBAR]\").toString())\n", " .setParent(FoobarName.ofProjectFoobarName(\"[PROJECT]\"," + " \"[FOOBAR]\").toString())\n", + " .setRequestId(\"requestId693933066\")\n", " .setSeverity(Severity.forNumber(0))\n", " .setFoobar(Foobar.newBuilder().build())\n", " .build();\n", diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientHeaderSampleComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientHeaderSampleComposerTest.java index bf33f30e26..265882ac3f 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientHeaderSampleComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientHeaderSampleComposerTest.java @@ -223,6 +223,7 @@ public void composeClassHeaderSample_firstMethodHasNoSignatures() { + " \"[FOOBAR]\").toString())\n", " .setParent(FoobarName.ofProjectFoobarName(\"[PROJECT]\"," + " \"[FOOBAR]\").toString())\n", + " .setRequestId(\"requestId693933066\")\n", " .setSeverity(Severity.forNumber(0))\n", " .setFoobar(Foobar.newBuilder().build())\n", " .build();\n", @@ -280,7 +281,11 @@ public void composeClassHeaderSample_firstMethodIsStream() { LineFormatter.lines( "try (EchoClient echoClient = EchoClient.create()) {\n", " ExpandRequest request =\n", - " ExpandRequest.newBuilder().setContent(\"content951530617\").setInfo(\"info3237038\").build();\n", + " ExpandRequest.newBuilder()\n", + " .setContent(\"content951530617\")\n", + " .setInfo(\"info3237038\")\n", + " .setRequestId(\"requestId693933066\")\n", + " .build();\n", " ServerStream stream = echoClient.expandCallable().call(request);\n", " for (EchoResponse response : stream) {\n", " // Do something when a response is received.\n", diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientMethodSampleComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientMethodSampleComposerTest.java index 6ba7985b5d..9839af8f31 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientMethodSampleComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientMethodSampleComposerTest.java @@ -335,6 +335,7 @@ public void valid_composeDefaultSample_pureUnaryReturnVoid() { + " \"[FOOBAR]\").toString())\n", " .setParent(FoobarName.ofProjectFoobarName(\"[PROJECT]\"," + " \"[FOOBAR]\").toString())\n", + " .setRequestId(\"requestId693933066\")\n", " .setSeverity(Severity.forNumber(0))\n", " .setFoobar(Foobar.newBuilder().build())\n", " .build();\n", @@ -397,6 +398,7 @@ public void valid_composeDefaultSample_pureUnaryReturnResponse() { + " \"[FOOBAR]\").toString())\n", " .setParent(FoobarName.ofProjectFoobarName(\"[PROJECT]\"," + " \"[FOOBAR]\").toString())\n", + " .setRequestId(\"requestId693933066\")\n", " .setSeverity(Severity.forNumber(0))\n", " .setFoobar(Foobar.newBuilder().build())\n", " .build();\n", diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java index 428b5156d9..ac78f38fc9 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java @@ -20,6 +20,7 @@ import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; +import com.google.api.FieldInfo.Format; import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.Reference; import com.google.api.generator.engine.ast.TypeNode; @@ -39,6 +40,8 @@ import com.google.showcase.v1beta1.EchoOuterClass; import com.google.showcase.v1beta1.TestingOuterClass; import com.google.testgapic.v1beta1.LockerProto; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; @@ -55,10 +58,18 @@ public class ParserTest { private ServiceDescriptor echoService; private FileDescriptor echoFileDescriptor; + private static final String YAML_DIRECTORY = "src/test/resources/"; + + private Optional serviceYamlProtoOpt; + @Before public void setUp() { echoFileDescriptor = EchoOuterClass.getDescriptor(); echoService = echoFileDescriptor.getServices().get(0); + String yamlFilename = "echo_v1beta1.yaml"; + Path yamlPath = Paths.get(YAML_DIRECTORY, yamlFilename); + serviceYamlProtoOpt = + ServiceYamlParser.parse(yamlPath.toString()); assertEquals("Echo", echoService.getName()); } @@ -121,6 +132,7 @@ public void parseMethods_basic() { messageTypes, resourceNames, Optional.empty(), + serviceYamlProtoOpt, outputResourceNames, Transport.GRPC); @@ -130,6 +142,8 @@ public void parseMethods_basic() { Method echoMethod = methods.get(0); assertEquals(echoMethod.name(), "Echo"); assertEquals(echoMethod.stream(), Method.Stream.NONE); + assertEquals(true, echoMethod.hasAutoPopulatedFields()); + assertEquals(Arrays.asList("request_id"), echoMethod.autoPopulatedFields()); // Detailed method signature parsing tests are in a separate unit test. List> methodSignatures = echoMethod.methodSignatures(); @@ -157,14 +171,17 @@ public void parseMethods_basic() { TypeNode.withReference(createStatusReference()), ImmutableList.of(), expandMethod.methodSignatures().get(0).get(1)); + assertEquals(false, expandMethod.hasAutoPopulatedFields()); Method collectMethod = methods.get(2); assertEquals("Collect", collectMethod.name()); assertEquals(Method.Stream.CLIENT, collectMethod.stream()); + assertEquals(false, collectMethod.hasAutoPopulatedFields()); Method chatMethod = methods.get(3); assertEquals("Chat", chatMethod.name()); assertEquals(Method.Stream.BIDI, chatMethod.stream()); + assertEquals(false, chatMethod.hasAutoPopulatedFields()); } @Test @@ -179,6 +196,7 @@ public void parseMethods_basicLro() { messageTypes, resourceNames, Optional.empty(), + Optional.empty(), outputResourceNames, Transport.GRPC); @@ -418,6 +436,26 @@ public void parseFields_mapType() { field.type()); } + @Test + public void parseFields_autoPopulated() { + Map messageTypes = Parser.parseMessages(echoFileDescriptor); + Message message = messageTypes.get("com.google.showcase.v1beta1.EchoRequest"); + Field field = message.fieldMap().get("request_id"); + assertEquals(true, field.isEligibleToBeAutoPopulated()); + assertEquals(Format.UUID4, field.fieldInfoFormat()); + field = message.fieldMap().get("name"); + assertEquals(false, field.isEligibleToBeAutoPopulated()); + assertEquals(null, field.fieldInfoFormat()); + field = message.fieldMap().get("severity"); + assertEquals(false, field.isEligibleToBeAutoPopulated()); + assertEquals(null, field.fieldInfoFormat()); + + message = messageTypes.get("com.google.showcase.v1beta1.ExpandRequest"); + field = message.fieldMap().get("request_id"); + assertEquals(false, field.isEligibleToBeAutoPopulated()); + assertEquals(Format.IPV6, field.fieldInfoFormat()); + } + @Test public void parseResourceNames_inputTypeHasReferenceNotInMethodSignature() { FileDescriptor testingFileDescriptor = TestingOuterClass.getDescriptor(); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ServiceYamlParserTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ServiceYamlParserTest.java index 8df9ddcf84..8847b7e6ef 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ServiceYamlParserTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ServiceYamlParserTest.java @@ -17,6 +17,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import com.google.api.MethodSettings; +import com.google.api.Publishing; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Optional; @@ -37,4 +39,22 @@ public void parseServiceYaml_basic() { com.google.api.Service serviceYamlProto = serviceYamlProtoOpt.get(); assertEquals("logging.googleapis.com", serviceYamlProto.getName()); } + + @Test + public void parseServiceYaml_autoPopulatedFields() { + String yamlFilename = "echo_v1beta1.yaml"; + Path yamlPath = Paths.get(YAML_DIRECTORY, yamlFilename); + Optional serviceYamlProtoOpt = + ServiceYamlParser.parse(yamlPath.toString()); + assertTrue(serviceYamlProtoOpt.isPresent()); + + com.google.api.Service serviceYamlProto = serviceYamlProtoOpt.get(); + assertEquals("showcase.googleapis.com", serviceYamlProto.getName()); + + Publishing publishingSettings = serviceYamlProto.getPublishing(); + java.util.List methodSettings = publishingSettings.getMethodSettingsList(); + MethodSettings methodSetting = methodSettings.get(0); + assertEquals("google.showcase.v1beta1.Echo.Echo", methodSetting.getSelector()); + assertEquals("request_id", methodSetting.getAutoPopulatedFieldsList().get(0)); + } } diff --git a/gapic-generator-java/src/test/proto/echo.proto b/gapic-generator-java/src/test/proto/echo.proto index ea262faf32..07dbf7cfe2 100644 --- a/gapic-generator-java/src/test/proto/echo.proto +++ b/gapic-generator-java/src/test/proto/echo.proto @@ -17,6 +17,7 @@ syntax = "proto3"; import "google/api/annotations.proto"; import "google/api/client.proto"; import "google/api/field_behavior.proto"; +import "google/api/field_info.proto"; import "google/api/resource.proto"; import "google/longrunning/operations.proto"; import "google/protobuf/duration.proto"; @@ -179,6 +180,12 @@ message EchoRequest { (google.api.field_behavior) = REQUIRED ]; + // TODO: @alicejli to remove this one auto-population of request-id is added officially to gapic-showcase. This is for testing purposes only. + // Based on go/client-populate-request-id-design; subject to change + string request_id = 7 [ + (google.api.field_info).format = UUID4 + ]; + oneof response { // The content to be echoed by the server. string content = 1; @@ -217,6 +224,12 @@ message ExpandRequest { google.rpc.Status error = 2; string info = 3; + + // TODO: @alicejli to remove this one auto-population of request-id is added officially to gapic-showcase. This is for testing purposes only. + // Based on go/client-populate-request-id-design; subject to change + string request_id = 4 [ + (google.api.field_info).format = IPV6 + ]; } // The request for the PagedExpand method. diff --git a/gapic-generator-java/src/test/resources/echo_v1beta1.yaml b/gapic-generator-java/src/test/resources/echo_v1beta1.yaml index 321758a4ea..07fb1fdb31 100644 --- a/gapic-generator-java/src/test/resources/echo_v1beta1.yaml +++ b/gapic-generator-java/src/test/resources/echo_v1beta1.yaml @@ -94,4 +94,9 @@ http: post: '/v1beta1/{name=operations/**}:cancel' additional_bindings: - post: '/v1beta2/{name=operations/**}:cancel' - - post: '/v1beta3/{name=operations/**}:cancel' \ No newline at end of file + - post: '/v1beta3/{name=operations/**}:cancel' +publishing: + method_settings: + - selector: google.showcase.v1beta1.Echo.Echo + auto_populated_fields: + - request_id \ No newline at end of file From 21d1fea629631774d95ddc408b6b8db2319b9438 Mon Sep 17 00:00:00 2001 From: Alice Li Date: Mon, 18 Dec 2023 17:00:29 -0500 Subject: [PATCH 02/14] fix lint --- .../com/google/api/generator/gapic/protoparser/ParserTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java index ac78f38fc9..f9155290c5 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java @@ -68,8 +68,7 @@ public void setUp() { echoService = echoFileDescriptor.getServices().get(0); String yamlFilename = "echo_v1beta1.yaml"; Path yamlPath = Paths.get(YAML_DIRECTORY, yamlFilename); - serviceYamlProtoOpt = - ServiceYamlParser.parse(yamlPath.toString()); + serviceYamlProtoOpt = ServiceYamlParser.parse(yamlPath.toString()); assertEquals("Echo", echoService.getName()); } From 3ea4c42aa8eebe5d059ab330b5a2d99afb4ade23 Mon Sep 17 00:00:00 2001 From: Alice Li Date: Mon, 18 Dec 2023 17:07:55 -0500 Subject: [PATCH 03/14] fix lint and update goldens --- .../api/generator/gapic/model/Field.java | 7 ++++-- .../api/generator/gapic/model/Method.java | 9 ++++--- .../generator/gapic/protoparser/Parser.java | 25 +++++++++++-------- .../composer/grpc/goldens/EchoClient.golden | 13 +++++++++- .../grpc/goldens/EchoClientTest.golden | 22 ++++++++++++++-- .../samples/echoclient/AsyncChat.golden | 1 + .../samples/echoclient/AsyncChatAgain.golden | 1 + .../samples/echoclient/AsyncCollect.golden | 1 + .../echoclient/AsyncCollideName.golden | 1 + .../samples/echoclient/AsyncEcho.golden | 1 + .../samples/echoclient/AsyncExpand.golden | 6 ++++- .../samples/echoclient/SyncCollideName.golden | 1 + .../samples/echoclient/SyncEcho.golden | 1 + 13 files changed, 70 insertions(+), 19 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java index a1cf0b8cc4..0eae0e1cce 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java @@ -33,13 +33,16 @@ public abstract class Field { public abstract TypeNode type(); - // If the field contains a google.api.field_info.format value of UUID4 and is not marked as REQUIRED, then it is eligible to be auto-populated if the customer does not set it. - // The field needs to be cross-referenced against Method.autoPopulatedFields() to make the final determination if it *should* be auto-populated. + // If the field contains a google.api.field_info.format value of UUID4 and is not marked as + // REQUIRED, then it is eligible to be auto-populated if the customer does not set it. + // The field needs to be cross-referenced against Method.autoPopulatedFields() to make the final + // determination if it *should* be auto-populated. // See go/client-populate-request-id-design for more details. public abstract boolean isEligibleToBeAutoPopulated(); @Nullable public abstract Format fieldInfoFormat(); + public abstract boolean isMessage(); public abstract boolean isEnum(); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Method.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Method.java index 6c94d917e9..f17d787825 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Method.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Method.java @@ -73,10 +73,13 @@ public boolean isPaged() { public abstract List autoPopulatedFields(); /** - * If a service's service_config.yaml file contains method_settings.auto_populated_fields for this method, and the method is a Unary-type, then this is true + * If a service's service_config.yaml file contains method_settings.auto_populated_fields for this + * method, and the method is a Unary-type, then this is true */ - public boolean hasAutoPopulatedFields(){ - return autoPopulatedFields() != null && autoPopulatedFields().size() > 0 && stream().equals(Stream.NONE); + public boolean hasAutoPopulatedFields() { + return autoPopulatedFields() != null + && autoPopulatedFields().size() > 0 + && stream().equals(Stream.NONE); } public abstract boolean operationPollingMethod(); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 211c5c82f0..7e6b8f5e25 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -695,18 +695,21 @@ static List parseMethods( List methods = new ArrayList<>(); // Parse the serviceYaml for autopopulated methods and fields once and put into a map - ImmutableMap.Builder >autoPopulatedMethodsWithFieldsBuilder = ImmutableMap.builder(); + ImmutableMap.Builder> autoPopulatedMethodsWithFieldsBuilder = + ImmutableMap.builder(); if (serviceYamlProtoOpt.isPresent()) { if (serviceYamlProtoOpt.get().getPublishing().getMethodSettingsList().size() > 0) { - for (MethodSettings methodSettings : serviceYamlProtoOpt.get().getPublishing() - .getMethodSettingsList()) { - if(methodSettings.getAutoPopulatedFieldsCount() > 0){ - autoPopulatedMethodsWithFieldsBuilder.put(methodSettings.getSelector(), methodSettings.getAutoPopulatedFieldsList()); + for (MethodSettings methodSettings : + serviceYamlProtoOpt.get().getPublishing().getMethodSettingsList()) { + if (methodSettings.getAutoPopulatedFieldsCount() > 0) { + autoPopulatedMethodsWithFieldsBuilder.put( + methodSettings.getSelector(), methodSettings.getAutoPopulatedFieldsList()); } } } } - ImmutableMap> autoPopulatedMethodsWithFields = autoPopulatedMethodsWithFieldsBuilder.build(); + ImmutableMap> autoPopulatedMethodsWithFields = + autoPopulatedMethodsWithFieldsBuilder.build(); for (MethodDescriptor protoMethod : serviceDescriptor.getMethods()) { // Parse the method. @@ -1031,16 +1034,18 @@ private static Field parseField( } } - // If field is annotated with fieldInfo.format = UUID4 and fieldBehavior != REQUIRED, then autopopulate the field - // TODO: Autopopulation requires fieldInfo.formatValue = UUID4; when more types are supported, this logic will need to be updated + // If field is annotated with fieldInfo.format = UUID4 and fieldBehavior != REQUIRED, then + // autopopulate the field + // TODO: Autopopulation requires fieldInfo.formatValue = UUID4; when more types are supported, + // this logic will need to be updated if (fieldOptions.hasExtension(FieldInfoProto.fieldInfo)) { fieldInfoFormat = fieldOptions.getExtension(FieldInfoProto.fieldInfo).getFormat(); if (fieldOptions.getExtension(FieldInfoProto.fieldInfo).getFormatValue() == 1) { isEligibleToBeAutoPopulated = true; } } - if (fieldOptions.getExtensionCount(FieldBehaviorProto.fieldBehavior) > 0){ - if(fieldOptions.getExtension(FieldBehaviorProto.fieldBehavior).contains(2)); + if (fieldOptions.getExtensionCount(FieldBehaviorProto.fieldBehavior) > 0) { + if (fieldOptions.getExtension(FieldBehaviorProto.fieldBehavior).contains(2)) ; isEligibleToBeAutoPopulated = false; } diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/EchoClient.golden b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/EchoClient.golden index 0abcdca6bb..8e92d7365d 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/EchoClient.golden +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/EchoClient.golden @@ -516,6 +516,7 @@ public class EchoClient implements BackgroundResource { * EchoRequest.newBuilder() * .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) * .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + * .setRequestId("requestId693933066") * .setSeverity(Severity.forNumber(0)) * .setFoobar(Foobar.newBuilder().build()) * .build(); @@ -545,6 +546,7 @@ public class EchoClient implements BackgroundResource { * EchoRequest.newBuilder() * .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) * .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + * .setRequestId("requestId693933066") * .setSeverity(Severity.forNumber(0)) * .setFoobar(Foobar.newBuilder().build()) * .build(); @@ -570,7 +572,11 @@ public class EchoClient implements BackgroundResource { * // https://cloud.google.com/java/docs/setup#configure_endpoints_for_the_client_library * try (EchoClient echoClient = EchoClient.create()) { * ExpandRequest request = - * ExpandRequest.newBuilder().setContent("content951530617").setInfo("info3237038").build(); + * ExpandRequest.newBuilder() + * .setContent("content951530617") + * .setInfo("info3237038") + * .setRequestId("requestId693933066") + * .build(); * ServerStream stream = echoClient.expandCallable().call(request); * for (EchoResponse response : stream) { * // Do something when a response is received. @@ -616,6 +622,7 @@ public class EchoClient implements BackgroundResource { * EchoRequest.newBuilder() * .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) * .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + * .setRequestId("requestId693933066") * .setSeverity(Severity.forNumber(0)) * .setFoobar(Foobar.newBuilder().build()) * .build(); @@ -643,6 +650,7 @@ public class EchoClient implements BackgroundResource { * EchoRequest.newBuilder() * .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) * .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + * .setRequestId("requestId693933066") * .setSeverity(Severity.forNumber(0)) * .setFoobar(Foobar.newBuilder().build()) * .build(); @@ -673,6 +681,7 @@ public class EchoClient implements BackgroundResource { * EchoRequest.newBuilder() * .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) * .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + * .setRequestId("requestId693933066") * .setSeverity(Severity.forNumber(0)) * .setFoobar(Foobar.newBuilder().build()) * .build(); @@ -1081,6 +1090,7 @@ public class EchoClient implements BackgroundResource { * EchoRequest.newBuilder() * .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) * .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + * .setRequestId("requestId693933066") * .setSeverity(Severity.forNumber(0)) * .setFoobar(Foobar.newBuilder().build()) * .build(); @@ -1110,6 +1120,7 @@ public class EchoClient implements BackgroundResource { * EchoRequest.newBuilder() * .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) * .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + * .setRequestId("requestId693933066") * .setSeverity(Severity.forNumber(0)) * .setFoobar(Foobar.newBuilder().build()) * .build(); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/EchoClientTest.golden b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/EchoClientTest.golden index 1372cdd94e..009d8688df 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/EchoClientTest.golden +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/EchoClientTest.golden @@ -108,6 +108,7 @@ public class EchoClientTest { EchoRequest.newBuilder() .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + .setRequestId("requestId693933066") .setSeverity(Severity.forNumber(0)) .setFoobar(Foobar.newBuilder().build()) .build(); @@ -403,7 +404,11 @@ public class EchoClientTest { .build(); mockEcho.addResponse(expectedResponse); ExpandRequest request = - ExpandRequest.newBuilder().setContent("content951530617").setInfo("info3237038").build(); + ExpandRequest.newBuilder() + .setContent("content951530617") + .setInfo("info3237038") + .setRequestId("requestId693933066") + .build(); MockStreamObserver responseObserver = new MockStreamObserver<>(); @@ -420,7 +425,11 @@ public class EchoClientTest { StatusRuntimeException exception = new StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT); mockEcho.addException(exception); ExpandRequest request = - ExpandRequest.newBuilder().setContent("content951530617").setInfo("info3237038").build(); + ExpandRequest.newBuilder() + .setContent("content951530617") + .setInfo("info3237038") + .setRequestId("requestId693933066") + .build(); MockStreamObserver responseObserver = new MockStreamObserver<>(); @@ -449,6 +458,7 @@ public class EchoClientTest { EchoRequest.newBuilder() .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + .setRequestId("requestId693933066") .setSeverity(Severity.forNumber(0)) .setFoobar(Foobar.newBuilder().build()) .build(); @@ -474,6 +484,7 @@ public class EchoClientTest { EchoRequest.newBuilder() .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + .setRequestId("requestId693933066") .setSeverity(Severity.forNumber(0)) .setFoobar(Foobar.newBuilder().build()) .build(); @@ -507,6 +518,7 @@ public class EchoClientTest { EchoRequest.newBuilder() .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + .setRequestId("requestId693933066") .setSeverity(Severity.forNumber(0)) .setFoobar(Foobar.newBuilder().build()) .build(); @@ -532,6 +544,7 @@ public class EchoClientTest { EchoRequest.newBuilder() .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + .setRequestId("requestId693933066") .setSeverity(Severity.forNumber(0)) .setFoobar(Foobar.newBuilder().build()) .build(); @@ -565,6 +578,7 @@ public class EchoClientTest { EchoRequest.newBuilder() .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + .setRequestId("requestId693933066") .setSeverity(Severity.forNumber(0)) .setFoobar(Foobar.newBuilder().build()) .build(); @@ -590,6 +604,7 @@ public class EchoClientTest { EchoRequest.newBuilder() .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + .setRequestId("requestId693933066") .setSeverity(Severity.forNumber(0)) .setFoobar(Foobar.newBuilder().build()) .build(); @@ -848,6 +863,7 @@ public class EchoClientTest { EchoRequest.newBuilder() .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + .setRequestId("requestId693933066") .setSeverity(Severity.forNumber(0)) .setFoobar(Foobar.newBuilder().build()) .build(); @@ -861,6 +877,7 @@ public class EchoClientTest { Assert.assertEquals(request.getName(), actualRequest.getName()); Assert.assertEquals(request.getParent(), actualRequest.getParent()); + Assert.assertEquals(request.getRequestId(), actualRequest.getRequestId()); Assert.assertEquals(request.getContent(), actualRequest.getContent()); Assert.assertEquals(request.getError(), actualRequest.getError()); Assert.assertEquals(request.getSeverity(), actualRequest.getSeverity()); @@ -881,6 +898,7 @@ public class EchoClientTest { EchoRequest.newBuilder() .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + .setRequestId("requestId693933066") .setSeverity(Severity.forNumber(0)) .setFoobar(Foobar.newBuilder().build()) .build(); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncChat.golden b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncChat.golden index 13867b1bb8..e17e9367c8 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncChat.golden +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncChat.golden @@ -43,6 +43,7 @@ public class AsyncChat { EchoRequest.newBuilder() .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + .setRequestId("requestId693933066") .setSeverity(Severity.forNumber(0)) .setFoobar(Foobar.newBuilder().build()) .build(); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncChatAgain.golden b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncChatAgain.golden index aba2c145d9..02e38bd9db 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncChatAgain.golden +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncChatAgain.golden @@ -43,6 +43,7 @@ public class AsyncChatAgain { EchoRequest.newBuilder() .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + .setRequestId("requestId693933066") .setSeverity(Severity.forNumber(0)) .setFoobar(Foobar.newBuilder().build()) .build(); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncCollect.golden b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncCollect.golden index 0103bdc4a5..cd8c21c72d 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncCollect.golden +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncCollect.golden @@ -61,6 +61,7 @@ public class AsyncCollect { EchoRequest.newBuilder() .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + .setRequestId("requestId693933066") .setSeverity(Severity.forNumber(0)) .setFoobar(Foobar.newBuilder().build()) .build(); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncCollideName.golden b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncCollideName.golden index f606fdf6ed..5ca5cd0bcc 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncCollideName.golden +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncCollideName.golden @@ -42,6 +42,7 @@ public class AsyncCollideName { EchoRequest.newBuilder() .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + .setRequestId("requestId693933066") .setSeverity(Severity.forNumber(0)) .setFoobar(Foobar.newBuilder().build()) .build(); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncEcho.golden b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncEcho.golden index 2c9d336813..087faaebf5 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncEcho.golden +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncEcho.golden @@ -42,6 +42,7 @@ public class AsyncEcho { EchoRequest.newBuilder() .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + .setRequestId("requestId693933066") .setSeverity(Severity.forNumber(0)) .setFoobar(Foobar.newBuilder().build()) .build(); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncExpand.golden b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncExpand.golden index bd0dbd67c3..aff30c88fc 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncExpand.golden +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/AsyncExpand.golden @@ -36,7 +36,11 @@ public class AsyncExpand { // https://cloud.google.com/java/docs/setup#configure_endpoints_for_the_client_library try (EchoClient echoClient = EchoClient.create()) { ExpandRequest request = - ExpandRequest.newBuilder().setContent("content951530617").setInfo("info3237038").build(); + ExpandRequest.newBuilder() + .setContent("content951530617") + .setInfo("info3237038") + .setRequestId("requestId693933066") + .build(); ServerStream stream = echoClient.expandCallable().call(request); for (EchoResponse response : stream) { // Do something when a response is received. diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/SyncCollideName.golden b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/SyncCollideName.golden index 57a842f907..f299524598 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/SyncCollideName.golden +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/SyncCollideName.golden @@ -41,6 +41,7 @@ public class SyncCollideName { EchoRequest.newBuilder() .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + .setRequestId("requestId693933066") .setSeverity(Severity.forNumber(0)) .setFoobar(Foobar.newBuilder().build()) .build(); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/SyncEcho.golden b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/SyncEcho.golden index f9fbef3fff..a0918f576d 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/SyncEcho.golden +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/samples/echoclient/SyncEcho.golden @@ -41,6 +41,7 @@ public class SyncEcho { EchoRequest.newBuilder() .setName(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) .setParent(FoobarName.ofProjectFoobarName("[PROJECT]", "[FOOBAR]").toString()) + .setRequestId("requestId693933066") .setSeverity(Severity.forNumber(0)) .setFoobar(Foobar.newBuilder().build()) .build(); From 9cd4d2085c218430ebdbf2077f6fda60ff4b89fa Mon Sep 17 00:00:00 2001 From: Alice Li Date: Tue, 19 Dec 2023 15:21:38 -0500 Subject: [PATCH 04/14] refactor --- .../api/generator/gapic/model/Field.java | 15 ++--- .../api/generator/gapic/model/Method.java | 7 +-- .../generator/gapic/protoparser/Parser.java | 61 +++++++++++-------- .../api/generator/gapic/model/MethodTest.java | 53 ++++++++++++++++ .../gapic/protoparser/ParserTest.java | 32 ++++++++-- .../protoparser/ServiceYamlParserTest.java | 3 +- .../src/test/proto/echo.proto | 5 +- 7 files changed, 128 insertions(+), 48 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java index 0eae0e1cce..5066c000e1 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java @@ -33,12 +33,8 @@ public abstract class Field { public abstract TypeNode type(); - // If the field contains a google.api.field_info.format value of UUID4 and is not marked as - // REQUIRED, then it is eligible to be auto-populated if the customer does not set it. - // The field needs to be cross-referenced against Method.autoPopulatedFields() to make the final - // determination if it *should* be auto-populated. - // See go/client-populate-request-id-design for more details. - public abstract boolean isEligibleToBeAutoPopulated(); + // If the field is annotated with google.api.field_behavior = REQUIRED, then this is true. + public abstract boolean isRequired(); @Nullable public abstract Format fieldInfoFormat(); @@ -83,7 +79,8 @@ public boolean equals(Object o) { return name().equals(other.name()) && originalName().equals(other.originalName()) && type().equals(other.type()) - && isEligibleToBeAutoPopulated() == other.isEligibleToBeAutoPopulated() + && isRequired() == other.isRequired() + && fieldInfoFormat() == other.fieldInfoFormat() && isMessage() == other.isMessage() && isEnum() == other.isEnum() && isRepeated() == other.isRepeated() @@ -113,7 +110,7 @@ public int hashCode() { public static Builder builder() { return new AutoValue_Field.Builder() - .setIsEligibleToBeAutoPopulated(false) + .setIsRequired(false) .setIsMessage(false) .setIsEnum(false) .setIsRepeated(false) @@ -130,7 +127,7 @@ public abstract static class Builder { public abstract Builder setType(TypeNode type); - public abstract Builder setIsEligibleToBeAutoPopulated(boolean isEligibleToBeAutoPopulated); + public abstract Builder setIsRequired(boolean isRequired); public abstract Builder setFieldInfoFormat(Format fieldInfoFormat); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Method.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Method.java index f17d787825..ee04e0f6f0 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Method.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Method.java @@ -17,6 +17,7 @@ import com.google.api.generator.engine.ast.TypeNode; import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; +import java.util.ArrayList; import java.util.List; import javax.annotation.Nullable; @@ -69,7 +70,6 @@ public boolean isPaged() { // [["content", "error"], ["content", "error", "info"]]. public abstract ImmutableList> methodSignatures(); - @Nullable public abstract List autoPopulatedFields(); /** @@ -77,9 +77,7 @@ public boolean isPaged() { * method, and the method is a Unary-type, then this is true */ public boolean hasAutoPopulatedFields() { - return autoPopulatedFields() != null - && autoPopulatedFields().size() > 0 - && stream().equals(Stream.NONE); + return autoPopulatedFields().size() > 0 && stream() == Stream.NONE; } public abstract boolean operationPollingMethod(); @@ -136,6 +134,7 @@ public boolean isSupportedByTransport(Transport transport) { public static Builder builder() { return new AutoValue_Method.Builder() .setStream(Stream.NONE) + .setAutoPopulatedFields(new ArrayList<>()) .setMethodSignatures(ImmutableList.of()) .setIsBatching(false) .setIsDeprecated(false) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 7e6b8f5e25..efc3dcad08 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -16,6 +16,7 @@ import com.google.api.ClientProto; import com.google.api.DocumentationRule; +import com.google.api.FieldBehavior; import com.google.api.FieldBehaviorProto; import com.google.api.FieldInfo.Format; import com.google.api.FieldInfoProto; @@ -695,21 +696,8 @@ static List parseMethods( List methods = new ArrayList<>(); // Parse the serviceYaml for autopopulated methods and fields once and put into a map - ImmutableMap.Builder> autoPopulatedMethodsWithFieldsBuilder = - ImmutableMap.builder(); - if (serviceYamlProtoOpt.isPresent()) { - if (serviceYamlProtoOpt.get().getPublishing().getMethodSettingsList().size() > 0) { - for (MethodSettings methodSettings : - serviceYamlProtoOpt.get().getPublishing().getMethodSettingsList()) { - if (methodSettings.getAutoPopulatedFieldsCount() > 0) { - autoPopulatedMethodsWithFieldsBuilder.put( - methodSettings.getSelector(), methodSettings.getAutoPopulatedFieldsList()); - } - } - } - } ImmutableMap> autoPopulatedMethodsWithFields = - autoPopulatedMethodsWithFieldsBuilder.build(); + parseAutoPopulatedMethodsAndFields(serviceYamlProtoOpt); for (MethodDescriptor protoMethod : serviceDescriptor.getMethods()) { // Parse the method. @@ -724,8 +712,8 @@ static List parseMethods( } } - // Parse the serviceYaml for autopopulated fields - List autoPopulatedFields = null; + // Associate the autopopulated fields with the correct method + List autoPopulatedFields = new ArrayList<>(); if (autoPopulatedMethodsWithFields.containsKey(protoMethod.getFullName())) { autoPopulatedFields = autoPopulatedMethodsWithFields.get(protoMethod.getFullName()); } @@ -1002,7 +990,7 @@ private static Field parseField( FieldOptions fieldOptions = fieldDescriptor.getOptions(); MessageOptions messageOptions = messageDescriptor.getOptions(); ResourceReference resourceReference = null; - boolean isEligibleToBeAutoPopulated = false; + boolean isRequired = false; Format fieldInfoFormat = null; if (fieldOptions.hasExtension(ResourceProto.resourceReference)) { com.google.api.ResourceReference protoResourceReference = @@ -1034,19 +1022,14 @@ private static Field parseField( } } - // If field is annotated with fieldInfo.format = UUID4 and fieldBehavior != REQUIRED, then - // autopopulate the field - // TODO: Autopopulation requires fieldInfo.formatValue = UUID4; when more types are supported, - // this logic will need to be updated if (fieldOptions.hasExtension(FieldInfoProto.fieldInfo)) { fieldInfoFormat = fieldOptions.getExtension(FieldInfoProto.fieldInfo).getFormat(); - if (fieldOptions.getExtension(FieldInfoProto.fieldInfo).getFormatValue() == 1) { - isEligibleToBeAutoPopulated = true; - } } if (fieldOptions.getExtensionCount(FieldBehaviorProto.fieldBehavior) > 0) { - if (fieldOptions.getExtension(FieldBehaviorProto.fieldBehavior).contains(2)) ; - isEligibleToBeAutoPopulated = false; + if (fieldOptions + .getExtension(FieldBehaviorProto.fieldBehavior) + .contains(FieldBehavior.REQUIRED)) ; + isRequired = true; } Field.Builder fieldBuilder = Field.builder(); @@ -1079,7 +1062,7 @@ private static Field parseField( fieldDescriptor.getContainingOneof() != null && fieldDescriptor.getContainingOneof().isSynthetic()) .setIsRepeated(fieldDescriptor.isRepeated()) - .setIsEligibleToBeAutoPopulated(isEligibleToBeAutoPopulated) + .setIsRequired(isRequired) .setFieldInfoFormat(fieldInfoFormat) .setIsMap(fieldDescriptor.isMapField()) .setResourceReference(resourceReference) @@ -1175,4 +1158,28 @@ static String parseNestedProtoTypeName(String fullyQualifiedName) { .collect(Collectors.toList()); return String.join(".", nestedTypeComponents); } + + /** + * Converts a serviceYaml file to a map of methods and autopopulated fields + * + * @param + */ + @VisibleForTesting + static ImmutableMap> parseAutoPopulatedMethodsAndFields( + Optional serviceYamlProtoOpt) { + ImmutableMap.Builder> autoPopulatedMethodsWithFieldsBuilder = + ImmutableMap.builder(); + if (serviceYamlProtoOpt.isPresent() + && serviceYamlProtoOpt.get().hasPublishing() + && serviceYamlProtoOpt.get().getPublishing().getMethodSettingsList().size() > 0) { + for (MethodSettings methodSettings : + serviceYamlProtoOpt.get().getPublishing().getMethodSettingsList()) { + if (methodSettings.getAutoPopulatedFieldsCount() > 0) { + autoPopulatedMethodsWithFieldsBuilder.put( + methodSettings.getSelector(), methodSettings.getAutoPopulatedFieldsList()); + } + } + } + return autoPopulatedMethodsWithFieldsBuilder.build(); + } } diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/model/MethodTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/model/MethodTest.java index 00754113ae..65c4f6039d 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/model/MethodTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/model/MethodTest.java @@ -15,14 +15,18 @@ package com.google.api.generator.gapic.model; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.gapic.model.HttpBindings.HttpBinding; import com.google.api.generator.gapic.model.HttpBindings.HttpVerb; +import com.google.api.generator.gapic.model.Method.Stream; import com.google.api.generator.gapic.model.RoutingHeaderRule.RoutingHeaderParam; import com.google.common.collect.ImmutableSet; +import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import org.junit.Test; public class MethodTest { @@ -114,6 +118,55 @@ public void shouldSetParamsExtractor_shouldReturnFalseIfHasNoHttpBindingsAndNoRo assertThat(method.shouldSetParamsExtractor()).isFalse(); } + @Test + public void hasAutoPopulatedFields_shouldReturnTrueIfMethodIsUnary() { + List autoPopulatedFields = Arrays.asList("field1", "field2"); + Method method = METHOD.toBuilder().setAutoPopulatedFields(autoPopulatedFields).build(); + method.toStream(false, false); + assertEquals(true, method.hasAutoPopulatedFields()); + } + + @Test + public void hasAutoPopulatedFields_shouldReturnFalseIfMethodIsStreaming() { + List autoPopulatedFields = Arrays.asList("field1", "field2"); + Method method = + METHOD + .toBuilder() + .setAutoPopulatedFields(autoPopulatedFields) + .setStream(Stream.SERVER) + .build(); + assertEquals(false, method.hasAutoPopulatedFields()); + + method = + METHOD + .toBuilder() + .setAutoPopulatedFields(autoPopulatedFields) + .setStream(Stream.BIDI) + .build(); + assertEquals(false, method.hasAutoPopulatedFields()); + + method = + METHOD + .toBuilder() + .setAutoPopulatedFields(autoPopulatedFields) + .setStream(Stream.CLIENT) + .build(); + assertEquals(false, method.hasAutoPopulatedFields()); + } + + @Test + public void hasAutoPopulatedFields_shouldReturnFalseIfAutoPopulatedFieldsIsEmpty() { + List autoPopulatedFields = new ArrayList<>(); + Method method = + METHOD + .toBuilder() + .setAutoPopulatedFields(autoPopulatedFields) + .setStream(Stream.NONE) + .build(); + method.toStream(false, false); + assertEquals(false, method.hasAutoPopulatedFields()); + } + @Test public void isSupportedByTransport_shouldReturnTrueIfHasHttpBindingsAndIsRESTEligibleForRESTTransport() { diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java index f9155290c5..e8050262d5 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java @@ -21,6 +21,7 @@ import static org.junit.Assert.assertTrue; import com.google.api.FieldInfo.Format; +import com.google.api.Service; import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.Reference; import com.google.api.generator.engine.ast.TypeNode; @@ -34,6 +35,7 @@ import com.google.api.generator.gapic.model.Transport; import com.google.bookshop.v1beta1.BookshopProto; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.MethodDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; @@ -440,21 +442,43 @@ public void parseFields_autoPopulated() { Map messageTypes = Parser.parseMessages(echoFileDescriptor); Message message = messageTypes.get("com.google.showcase.v1beta1.EchoRequest"); Field field = message.fieldMap().get("request_id"); - assertEquals(true, field.isEligibleToBeAutoPopulated()); + assertEquals(false, field.isRequired()); assertEquals(Format.UUID4, field.fieldInfoFormat()); field = message.fieldMap().get("name"); - assertEquals(false, field.isEligibleToBeAutoPopulated()); + assertEquals(true, field.isRequired()); assertEquals(null, field.fieldInfoFormat()); field = message.fieldMap().get("severity"); - assertEquals(false, field.isEligibleToBeAutoPopulated()); + assertEquals(false, field.isRequired()); assertEquals(null, field.fieldInfoFormat()); message = messageTypes.get("com.google.showcase.v1beta1.ExpandRequest"); field = message.fieldMap().get("request_id"); - assertEquals(false, field.isEligibleToBeAutoPopulated()); + assertEquals(false, field.isRequired()); assertEquals(Format.IPV6, field.fieldInfoFormat()); } + @Test + public void parseAutoPopulatedMethodsAndFields_exists() { + ImmutableMap> autoPopulatedMethodsWithFields = + Parser.parseAutoPopulatedMethodsAndFields(serviceYamlProtoOpt); + assertEquals( + true, autoPopulatedMethodsWithFields.containsKey("google.showcase.v1beta1.Echo.Echo")); + assertEquals( + Arrays.asList("request_id"), + autoPopulatedMethodsWithFields.get("google.showcase.v1beta1.Echo.Echo")); + } + + @Test + public void parseAutoPopulatedMethodsAndFields_doesNotExist() { + String yamlFilename = "logging.yaml"; + Path yamlPath = Paths.get(YAML_DIRECTORY, yamlFilename); + Optional serviceYamlProtoOpt_Null = ServiceYamlParser.parse(yamlPath.toString()); + + ImmutableMap> autoPopulatedMethodsWithFields = + Parser.parseAutoPopulatedMethodsAndFields(serviceYamlProtoOpt_Null); + assertEquals(true, autoPopulatedMethodsWithFields.isEmpty()); + } + @Test public void parseResourceNames_inputTypeHasReferenceNotInMethodSignature() { FileDescriptor testingFileDescriptor = TestingOuterClass.getDescriptor(); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ServiceYamlParserTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ServiceYamlParserTest.java index 8847b7e6ef..9aa2aebb81 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ServiceYamlParserTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ServiceYamlParserTest.java @@ -21,6 +21,7 @@ import com.google.api.Publishing; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.List; import java.util.Optional; import org.junit.Test; @@ -52,7 +53,7 @@ public void parseServiceYaml_autoPopulatedFields() { assertEquals("showcase.googleapis.com", serviceYamlProto.getName()); Publishing publishingSettings = serviceYamlProto.getPublishing(); - java.util.List methodSettings = publishingSettings.getMethodSettingsList(); + List methodSettings = publishingSettings.getMethodSettingsList(); MethodSettings methodSetting = methodSettings.get(0); assertEquals("google.showcase.v1beta1.Echo.Echo", methodSetting.getSelector()); assertEquals("request_id", methodSetting.getAutoPopulatedFieldsList().get(0)); diff --git a/gapic-generator-java/src/test/proto/echo.proto b/gapic-generator-java/src/test/proto/echo.proto index 07dbf7cfe2..41bbe2bd40 100644 --- a/gapic-generator-java/src/test/proto/echo.proto +++ b/gapic-generator-java/src/test/proto/echo.proto @@ -180,8 +180,7 @@ message EchoRequest { (google.api.field_behavior) = REQUIRED ]; - // TODO: @alicejli to remove this one auto-population of request-id is added officially to gapic-showcase. This is for testing purposes only. - // Based on go/client-populate-request-id-design; subject to change + // This field is added based on go/client-populate-request-id-design; subject to change string request_id = 7 [ (google.api.field_info).format = UUID4 ]; @@ -226,7 +225,7 @@ message ExpandRequest { string info = 3; // TODO: @alicejli to remove this one auto-population of request-id is added officially to gapic-showcase. This is for testing purposes only. - // Based on go/client-populate-request-id-design; subject to change + // This field is added based on go/client-populate-request-id-design; subject to change string request_id = 4 [ (google.api.field_info).format = IPV6 ]; From 61db4c8d081c33cef69a76a1dafefc434267e253 Mon Sep 17 00:00:00 2001 From: Alice Li Date: Tue, 19 Dec 2023 15:22:48 -0500 Subject: [PATCH 05/14] add todo --- .../com/google/api/generator/gapic/protoparser/Parser.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index efc3dcad08..2446d29663 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -1161,8 +1161,7 @@ static String parseNestedProtoTypeName(String fullyQualifiedName) { /** * Converts a serviceYaml file to a map of methods and autopopulated fields - * - * @param + * TODO: Confirm whether or not wildcards need to be supported. If so, update this logic. */ @VisibleForTesting static ImmutableMap> parseAutoPopulatedMethodsAndFields( From 4bd1019c85222a4a1030e503794cdb79e4e6fffd Mon Sep 17 00:00:00 2001 From: Alice Li Date: Tue, 19 Dec 2023 15:26:49 -0500 Subject: [PATCH 06/14] fix lint --- .../com/google/api/generator/gapic/protoparser/Parser.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 2446d29663..94afe0f12d 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -1160,8 +1160,8 @@ static String parseNestedProtoTypeName(String fullyQualifiedName) { } /** - * Converts a serviceYaml file to a map of methods and autopopulated fields - * TODO: Confirm whether or not wildcards need to be supported. If so, update this logic. + * Converts a serviceYaml file to a map of methods and autopopulated fields TODO: Confirm whether + * or not wildcards need to be supported. If so, update this logic. */ @VisibleForTesting static ImmutableMap> parseAutoPopulatedMethodsAndFields( From 3199f92dde44e05e5cb9aa4646116f6e94c2c7cd Mon Sep 17 00:00:00 2001 From: Alice Li Date: Wed, 20 Dec 2023 11:39:03 -0500 Subject: [PATCH 07/14] update comment --- .../main/java/com/google/api/generator/gapic/model/Field.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java index 5066c000e1..297e12d78c 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java @@ -33,7 +33,9 @@ public abstract class Field { public abstract TypeNode type(); - // If the field is annotated with google.api.field_behavior = REQUIRED, then this is true. + // If the field is annotated with google.api.field_behavior = REQUIRED, then this is true. This is + // currently only used to check if a field should be auto-populated. If it is true, then it should + // *not* be autopopulated. public abstract boolean isRequired(); @Nullable From 7dd2122a4e5e7744bfce321ad9e5efde2a71559d Mon Sep 17 00:00:00 2001 From: Alice Li Date: Thu, 21 Dec 2023 13:54:39 -0500 Subject: [PATCH 08/14] add unit tests --- .../api/generator/gapic/model/Field.java | 2 + .../generator/gapic/protoparser/Parser.java | 34 ++++---- .../gapic/protoparser/ParserTest.java | 79 ++++++++++++++++++- 3 files changed, 95 insertions(+), 20 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java index 297e12d78c..6a21ad1ad6 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java @@ -100,6 +100,8 @@ public int hashCode() { + 19 * type().hashCode() + (isMessage() ? 1 : 0) * 23 + (isEnum() ? 1 : 0) * 29 + + (isRequired() ? 1 : 0) * 31 + + 31 * fieldInfoFormat().hashCode() + (isRepeated() ? 1 : 0) * 31 + (isMap() ? 1 : 0) * 37 + (isContainedInOneof() ? 1 : 0) * 41 diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 94afe0f12d..234e3a5bed 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -696,7 +696,7 @@ static List parseMethods( List methods = new ArrayList<>(); // Parse the serviceYaml for autopopulated methods and fields once and put into a map - ImmutableMap> autoPopulatedMethodsWithFields = + Map> autoPopulatedMethodsWithFields = parseAutoPopulatedMethodsAndFields(serviceYamlProtoOpt); for (MethodDescriptor protoMethod : serviceDescriptor.getMethods()) { @@ -1160,25 +1160,25 @@ static String parseNestedProtoTypeName(String fullyQualifiedName) { } /** - * Converts a serviceYaml file to a map of methods and autopopulated fields TODO: Confirm whether - * or not wildcards need to be supported. If so, update this logic. + * Converts a serviceYaml file to a map of methods and autopopulated fields. Note: this does NOT + * currently support wildcards in MethodSettings.selectors. */ @VisibleForTesting - static ImmutableMap> parseAutoPopulatedMethodsAndFields( + static Map> parseAutoPopulatedMethodsAndFields( Optional serviceYamlProtoOpt) { - ImmutableMap.Builder> autoPopulatedMethodsWithFieldsBuilder = - ImmutableMap.builder(); - if (serviceYamlProtoOpt.isPresent() - && serviceYamlProtoOpt.get().hasPublishing() - && serviceYamlProtoOpt.get().getPublishing().getMethodSettingsList().size() > 0) { - for (MethodSettings methodSettings : - serviceYamlProtoOpt.get().getPublishing().getMethodSettingsList()) { - if (methodSettings.getAutoPopulatedFieldsCount() > 0) { - autoPopulatedMethodsWithFieldsBuilder.put( - methodSettings.getSelector(), methodSettings.getAutoPopulatedFieldsList()); - } - } + if (!hasMethodSettings(serviceYamlProtoOpt)) { + return ImmutableMap.>builder().build(); } - return autoPopulatedMethodsWithFieldsBuilder.build(); + return serviceYamlProtoOpt.get().getPublishing().getMethodSettingsList().stream() + .collect( + Collectors.toMap( + MethodSettings::getSelector, MethodSettings::getAutoPopulatedFieldsList)); + } + + @VisibleForTesting + static boolean hasMethodSettings(Optional serviceYamlProtoOpt) { + return serviceYamlProtoOpt.isPresent() + && serviceYamlProtoOpt.get().hasPublishing() + && !serviceYamlProtoOpt.get().getPublishing().getMethodSettingsList().isEmpty(); } } diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java index e8050262d5..2d071e4a8b 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java @@ -21,6 +21,8 @@ import static org.junit.Assert.assertTrue; import com.google.api.FieldInfo.Format; +import com.google.api.MethodSettings; +import com.google.api.Publishing; import com.google.api.Service; import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.Reference; @@ -35,7 +37,6 @@ import com.google.api.generator.gapic.model.Transport; import com.google.bookshop.v1beta1.BookshopProto; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.MethodDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; @@ -459,7 +460,7 @@ public void parseFields_autoPopulated() { @Test public void parseAutoPopulatedMethodsAndFields_exists() { - ImmutableMap> autoPopulatedMethodsWithFields = + Map> autoPopulatedMethodsWithFields = Parser.parseAutoPopulatedMethodsAndFields(serviceYamlProtoOpt); assertEquals( true, autoPopulatedMethodsWithFields.containsKey("google.showcase.v1beta1.Echo.Echo")); @@ -474,11 +475,83 @@ public void parseAutoPopulatedMethodsAndFields_doesNotExist() { Path yamlPath = Paths.get(YAML_DIRECTORY, yamlFilename); Optional serviceYamlProtoOpt_Null = ServiceYamlParser.parse(yamlPath.toString()); - ImmutableMap> autoPopulatedMethodsWithFields = + Map> autoPopulatedMethodsWithFields = Parser.parseAutoPopulatedMethodsAndFields(serviceYamlProtoOpt_Null); assertEquals(true, autoPopulatedMethodsWithFields.isEmpty()); } + @Test + public void parseAutoPopulatedMethodsAndFields_returnsEmptyMapIfServiceYamlIsNull() { + assertEquals(true, Parser.parseAutoPopulatedMethodsAndFields(Optional.empty()).isEmpty()); + } + + @Test + public void parseAutoPopulatedMethodsAndFields_returnsMapOfMethodsAndAutoPopulatedFields() { + MethodSettings testMethodSettings = + MethodSettings.newBuilder() + .setSelector("test_method") + .addAutoPopulatedFields("test_field") + .addAutoPopulatedFields("test_field_2") + .build(); + MethodSettings testMethodSettings2 = + MethodSettings.newBuilder() + .setSelector("test_method_2") + .addAutoPopulatedFields("test_field_3") + .build(); + MethodSettings testMethodSettings3 = + MethodSettings.newBuilder().setSelector("test_method_3").build(); + Publishing testPublishing = + Publishing.newBuilder() + .addMethodSettings(testMethodSettings) + .addMethodSettings(testMethodSettings2) + .addMethodSettings(testMethodSettings3) + .build(); + Optional testService = + Optional.of(Service.newBuilder().setPublishing(testPublishing).build()); + assertEquals( + Arrays.asList("test_field", "test_field_2"), + Parser.parseAutoPopulatedMethodsAndFields(testService).get("test_method")); + assertEquals( + Arrays.asList("test_field_3"), + Parser.parseAutoPopulatedMethodsAndFields(testService).get("test_method_2")); + assertEquals( + Arrays.asList(), + Parser.parseAutoPopulatedMethodsAndFields(testService).get("test_method_3")); + assertEquals( + false, Parser.parseAutoPopulatedMethodsAndFields(testService).containsKey("test_method_4")); + } + + @Test + public void hasMethodSettings_shouldReturnFalseIfServiceYamlDoesNotExist() { + assertEquals(false, Parser.hasMethodSettings(Optional.empty())); + } + + @Test + public void hasMethodSettings_shouldReturnFalseIfServiceYamlDoesNotHavePublishing() { + assertEquals(false, Parser.hasMethodSettings(Optional.of(Service.newBuilder().build()))); + } + + @Test + public void hasMethodSettings_shouldReturnFalseIfServiceYamlHasEmptyMethodSettings() { + assertEquals( + false, + Parser.hasMethodSettings( + Optional.of( + Service.newBuilder().setPublishing(Publishing.newBuilder().build()).build()))); + } + + @Test + public void hasMethodSettings_shouldReturnTrueIfServiceYamlHasNonEmptyMethodSettings() { + MethodSettings testMethodSettings = + MethodSettings.newBuilder().setSelector("test_method").build(); + Publishing testPublishing = + Publishing.newBuilder().addMethodSettings(testMethodSettings).build(); + assertEquals( + true, + Parser.hasMethodSettings( + Optional.of(Service.newBuilder().setPublishing(testPublishing).build()))); + } + @Test public void parseResourceNames_inputTypeHasReferenceNotInMethodSignature() { FileDescriptor testingFileDescriptor = TestingOuterClass.getDescriptor(); From d2bf821c5280cfd74d09bc0b5b668bf8342ab63b Mon Sep 17 00:00:00 2001 From: Alice Li Date: Thu, 21 Dec 2023 14:03:50 -0500 Subject: [PATCH 09/14] update hashcode() --- .../main/java/com/google/api/generator/gapic/model/Field.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java index 6a21ad1ad6..7f91a2a7ba 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java @@ -101,7 +101,7 @@ public int hashCode() { + (isMessage() ? 1 : 0) * 23 + (isEnum() ? 1 : 0) * 29 + (isRequired() ? 1 : 0) * 31 - + 31 * fieldInfoFormat().hashCode() + + (fieldInfoFormat() == null ? 0 : fieldInfoFormat().hashCode()) + (isRepeated() ? 1 : 0) * 31 + (isMap() ? 1 : 0) * 37 + (isContainedInOneof() ? 1 : 0) * 41 From f3a77798b44e6d77434978b351607ba214033639 Mon Sep 17 00:00:00 2001 From: Alice Li Date: Thu, 28 Dec 2023 11:15:35 -0500 Subject: [PATCH 10/14] update comments --- .../main/java/com/google/api/generator/gapic/model/Field.java | 3 ++- .../main/java/com/google/api/generator/gapic/model/Method.java | 2 +- gapic-generator-java/src/test/proto/echo.proto | 1 - gapic-generator-java/src/test/resources/echo_v1beta1.yaml | 1 + 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java index 7f91a2a7ba..997ea54e5a 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Field.java @@ -34,7 +34,8 @@ public abstract class Field { public abstract TypeNode type(); // If the field is annotated with google.api.field_behavior = REQUIRED, then this is true. This is - // currently only used to check if a field should be auto-populated. If it is true, then it should + // currently only used to check if a field should be auto-populated. If it is true, then the field + // should // *not* be autopopulated. public abstract boolean isRequired(); diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Method.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Method.java index ee04e0f6f0..f8f815cc08 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Method.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/model/Method.java @@ -77,7 +77,7 @@ public boolean isPaged() { * method, and the method is a Unary-type, then this is true */ public boolean hasAutoPopulatedFields() { - return autoPopulatedFields().size() > 0 && stream() == Stream.NONE; + return !autoPopulatedFields().isEmpty() && stream() == Stream.NONE; } public abstract boolean operationPollingMethod(); diff --git a/gapic-generator-java/src/test/proto/echo.proto b/gapic-generator-java/src/test/proto/echo.proto index 41bbe2bd40..d963447340 100644 --- a/gapic-generator-java/src/test/proto/echo.proto +++ b/gapic-generator-java/src/test/proto/echo.proto @@ -224,7 +224,6 @@ message ExpandRequest { string info = 3; - // TODO: @alicejli to remove this one auto-population of request-id is added officially to gapic-showcase. This is for testing purposes only. // This field is added based on go/client-populate-request-id-design; subject to change string request_id = 4 [ (google.api.field_info).format = IPV6 diff --git a/gapic-generator-java/src/test/resources/echo_v1beta1.yaml b/gapic-generator-java/src/test/resources/echo_v1beta1.yaml index 07fb1fdb31..57d9f90115 100644 --- a/gapic-generator-java/src/test/resources/echo_v1beta1.yaml +++ b/gapic-generator-java/src/test/resources/echo_v1beta1.yaml @@ -97,6 +97,7 @@ http: - post: '/v1beta3/{name=operations/**}:cancel' publishing: method_settings: + # TODO: Add more test cases for scenarios where the field does not exist, the field is not a String, etc. Eventually the API Linter should handle some of those cases. - selector: google.showcase.v1beta1.Echo.Echo auto_populated_fields: - request_id \ No newline at end of file From f677beed1aabf3d4b6e74e69a3807f7d11b26685 Mon Sep 17 00:00:00 2001 From: Alice Li Date: Thu, 28 Dec 2023 14:54:34 -0500 Subject: [PATCH 11/14] remove empty check --- .../com/google/api/generator/gapic/protoparser/Parser.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 234e3a5bed..1bbcc85fdb 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -1178,7 +1178,6 @@ static Map> parseAutoPopulatedMethodsAndFields( @VisibleForTesting static boolean hasMethodSettings(Optional serviceYamlProtoOpt) { return serviceYamlProtoOpt.isPresent() - && serviceYamlProtoOpt.get().hasPublishing() - && !serviceYamlProtoOpt.get().getPublishing().getMethodSettingsList().isEmpty(); + && serviceYamlProtoOpt.get().hasPublishing(); } } From e89f54d9d999cd97eda06549cfe64383b29d93a4 Mon Sep 17 00:00:00 2001 From: Alice Li Date: Thu, 28 Dec 2023 14:57:29 -0500 Subject: [PATCH 12/14] remove unit test --- .../api/generator/gapic/protoparser/ParserTest.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java index 2d071e4a8b..16340e0a6b 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java @@ -531,15 +531,6 @@ public void hasMethodSettings_shouldReturnFalseIfServiceYamlDoesNotHavePublishin assertEquals(false, Parser.hasMethodSettings(Optional.of(Service.newBuilder().build()))); } - @Test - public void hasMethodSettings_shouldReturnFalseIfServiceYamlHasEmptyMethodSettings() { - assertEquals( - false, - Parser.hasMethodSettings( - Optional.of( - Service.newBuilder().setPublishing(Publishing.newBuilder().build()).build()))); - } - @Test public void hasMethodSettings_shouldReturnTrueIfServiceYamlHasNonEmptyMethodSettings() { MethodSettings testMethodSettings = From 5ecdfbb31f6269e69dd6ce23fd5ef982eb03f3cc Mon Sep 17 00:00:00 2001 From: Alice Li Date: Thu, 28 Dec 2023 15:05:18 -0500 Subject: [PATCH 13/14] fix lint --- .../com/google/api/generator/gapic/protoparser/Parser.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 1bbcc85fdb..9f1b395940 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -1177,7 +1177,6 @@ static Map> parseAutoPopulatedMethodsAndFields( @VisibleForTesting static boolean hasMethodSettings(Optional serviceYamlProtoOpt) { - return serviceYamlProtoOpt.isPresent() - && serviceYamlProtoOpt.get().hasPublishing(); + return serviceYamlProtoOpt.isPresent() && serviceYamlProtoOpt.get().hasPublishing(); } } From c4c2a4b646e7ebc92270e36a3bf014148f1be0eb Mon Sep 17 00:00:00 2001 From: Alice Li Date: Thu, 28 Dec 2023 15:18:11 -0500 Subject: [PATCH 14/14] add todo --- .../api/generator/gapic/protoparser/ServiceYamlParserTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ServiceYamlParserTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ServiceYamlParserTest.java index 9aa2aebb81..f50fc572ff 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ServiceYamlParserTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ServiceYamlParserTest.java @@ -41,6 +41,8 @@ public void parseServiceYaml_basic() { assertEquals("logging.googleapis.com", serviceYamlProto.getName()); } + // TODO: Add more scenarios (e.g. null MethodSettings, null PublishingSettings, incorrect + // FieldNames, etc.) @Test public void parseServiceYaml_autoPopulatedFields() { String yamlFilename = "echo_v1beta1.yaml";