From 3dffa8d11e05d5248fd499bfd21bdee20f37fae0 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Mon, 31 Aug 2020 13:55:43 -0700 Subject: [PATCH 1/3] feat: parse batching descriptor fields --- .../gapic/model/GapicBatchingSettings.java | 17 ++++++++++ .../BatchingSettingsConfigParser.java | 34 +++++++++++++++++++ .../composer/RetrySettingsComposerTest.java | 5 +++ .../gapic/model/GapicServiceConfigTest.java | 2 ++ .../BatchingSettingsConfigParserTest.java | 8 +++++ 5 files changed, 66 insertions(+) diff --git a/src/main/java/com/google/api/generator/gapic/model/GapicBatchingSettings.java b/src/main/java/com/google/api/generator/gapic/model/GapicBatchingSettings.java index 349cb479ab..2f7d4de00d 100644 --- a/src/main/java/com/google/api/generator/gapic/model/GapicBatchingSettings.java +++ b/src/main/java/com/google/api/generator/gapic/model/GapicBatchingSettings.java @@ -15,6 +15,8 @@ package com.google.api.generator.gapic.model; import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; +import java.util.List; import javax.annotation.Nullable; @AutoValue @@ -25,6 +27,7 @@ public enum FlowControlLimitExceededBehavior { IGNORE }; + // Threshold fields. public abstract String protoPakkage(); public abstract String serviceName(); @@ -45,6 +48,14 @@ public enum FlowControlLimitExceededBehavior { public abstract FlowControlLimitExceededBehavior flowControlLimitExceededBehavior(); + // Batch descriptor fields. + public abstract String batchedFieldName(); + + public abstract ImmutableList discriminatorFieldNames(); + + @Nullable + public abstract String subresponseFieldName(); + public boolean matches(Service service, Method method) { return protoPakkage().equals(service.protoPakkage()) && serviceName().equals(service.name()) @@ -77,6 +88,12 @@ public abstract static class Builder { public abstract Builder setFlowControlLimitExceededBehavior( FlowControlLimitExceededBehavior behavior); + public abstract Builder setBatchedFieldName(String batchedFieldName); + + public abstract Builder setDiscriminatorFieldNames(List discriminatorFieldNames); + + public abstract Builder setSubresponseFieldName(String subresponseFieldName); + public abstract GapicBatchingSettings build(); } } diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParser.java index 721b79beea..79a1c9817d 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParser.java @@ -38,6 +38,8 @@ public class BatchingSettingsConfigParser { private static String YAML_KEY_METHODS = "methods"; private static String YAML_KEY_BATCHING = "batching"; private static String YAML_KEY_THRESHOLDS = "thresholds"; + private static String YAML_KEY_DESCRIPTOR = "batch_descriptor"; + private static String YAML_KEY_BATCHING_ELEMENT_COUNT_THRESHOLD = "element_count_threshold"; private static String YAML_KEY_BATCHING_DELAY_THRESHOLD_MILLIS = "delay_threshold_millis"; private static String YAML_KEY_BATCHING_REQUEST_BYTE_THRESHOLD = "request_byte_threshold"; @@ -46,6 +48,10 @@ public class BatchingSettingsConfigParser { private static String YAML_KEY_BATCHING_FLOW_CONTROL_LIMIT_EXCEEDED_BEHAVIOR = "flow_control_limit_exceeded_behavior"; + private static String YAML_KEY_DESCRIPTOR_BATCHED_FIELD = "batched_field"; + private static String YAML_KEY_DESCRIPTOR_DISCRIMINATOR_FIELD = "discriminator_fields"; + private static String YAML_KEY_DESCRIPTOR_SUBRESPONSE_FIELD = "subresponse_field"; + public static Optional> parse( Optional gapicYamlConfigFilePathOpt) { return gapicYamlConfigFilePathOpt.isPresent() @@ -94,6 +100,13 @@ private static Optional> parseFromMap(Map batchingYamlConfig = (Map) batchingOuterYamlConfig.get(YAML_KEY_THRESHOLDS); Preconditions.checkState( @@ -147,6 +160,27 @@ private static Optional> parseFromMap(Map descriptorYamlConfig = + (Map) batchingOuterYamlConfig.get(YAML_KEY_DESCRIPTOR); + Preconditions.checkState( + descriptorYamlConfig.containsKey(YAML_KEY_DESCRIPTOR_BATCHED_FIELD) + && descriptorYamlConfig.containsKey(YAML_KEY_DESCRIPTOR_DISCRIMINATOR_FIELD), + String.format( + "Batching descriptor YAML config is missing one of %s or %s fields", + YAML_KEY_DESCRIPTOR_BATCHED_FIELD, YAML_KEY_DESCRIPTOR_DISCRIMINATOR_FIELD)); + + settingsBuilder.setBatchedFieldName( + (String) descriptorYamlConfig.get(YAML_KEY_DESCRIPTOR_BATCHED_FIELD)); + settingsBuilder.setDiscriminatorFieldNames( + (List) descriptorYamlConfig.get(YAML_KEY_DESCRIPTOR_DISCRIMINATOR_FIELD)); + + if (descriptorYamlConfig.containsKey(YAML_KEY_DESCRIPTOR_SUBRESPONSE_FIELD)) { + settingsBuilder.setSubresponseFieldName( + (String) descriptorYamlConfig.get(YAML_KEY_DESCRIPTOR_SUBRESPONSE_FIELD)); + } + settings.add(settingsBuilder.build()); } } diff --git a/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java index b9c02053a3..27eedd2d68 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java @@ -409,6 +409,9 @@ public void batchingSettings_minimalFlowControlSettings() { .setElementCountThreshold(100) .setRequestByteThreshold(1048576) .setDelayThresholdMillis(10) + .setBatchedFieldName("messages") + .setDiscriminatorFieldNames(Arrays.asList("topic")) + .setSubresponseFieldName("message_ids") .build(); Expr builderExpr = @@ -487,6 +490,8 @@ public void batchingSettings_fullFlowControlSettings() { .setFlowControlByteLimit(10485760) .setFlowControlLimitExceededBehavior( GapicBatchingSettings.FlowControlLimitExceededBehavior.THROW_EXCEPTION) + .setBatchedFieldName("entries") + .setDiscriminatorFieldNames(Arrays.asList("log_name", "resource", "labels")) .build(); Expr builderExpr = diff --git a/src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java b/src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java index 6b6618a2dd..7d1ba5c9d5 100644 --- a/src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java +++ b/src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java @@ -147,6 +147,8 @@ public void serviceConfig_withBatchingSettings() { .setElementCountThreshold(1000) .setRequestByteThreshold(2000) .setDelayThresholdMillis(3000) + .setBatchedFieldName("name") + .setDiscriminatorFieldNames(Arrays.asList("severity")) .build(); Optional> batchingSettingsOpt = Optional.of(Arrays.asList(origBatchingSetting)); diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParserTest.java index ed420d87a2..92ba2e5094 100644 --- a/src/test/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParserTest.java +++ b/src/test/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParserTest.java @@ -75,6 +75,10 @@ public void parseBatchingSettings_logging() { assertEquals( GapicBatchingSettings.FlowControlLimitExceededBehavior.THROW_EXCEPTION, setting.flowControlLimitExceededBehavior()); + + assertEquals("entries", setting.batchedFieldName()); + assertThat(setting.discriminatorFieldNames()).containsExactly("log_name", "resource", "labels"); + assertThat(setting.subresponseFieldName()).isNull(); } @Test @@ -102,5 +106,9 @@ public void parseBatchingSettings_pubsub() { assertEquals( GapicBatchingSettings.FlowControlLimitExceededBehavior.IGNORE, setting.flowControlLimitExceededBehavior()); + + assertEquals("messages", setting.batchedFieldName()); + assertThat(setting.discriminatorFieldNames()).containsExactly("topic"); + assertEquals("message_ids", setting.subresponseFieldName()); } } From c656ad1e79eee5d3aa631b91a738709d2495adcb Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Mon, 31 Aug 2020 14:13:01 -0700 Subject: [PATCH 2/3] feat: add Field.isMap and map-parsing test --- .../api/generator/gapic/model/Field.java | 6 +++- .../generator/gapic/protoparser/Parser.java | 1 + .../gapic/protoparser/TypeParser.java | 8 ++--- .../gapic/protoparser/ParserTest.java | 20 +++++++++++++ .../generator/gapic/testdata/testing.proto | 30 ++++++++++--------- 5 files changed, 46 insertions(+), 19 deletions(-) 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 0d26f33b09..8f6d10b2af 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 @@ -26,6 +26,8 @@ public abstract class Field { public abstract boolean isRepeated(); + public abstract boolean isMap(); + @Nullable public abstract ResourceReference resourceReference(); @@ -34,7 +36,7 @@ public boolean hasResourceReference() { } public static Builder builder() { - return new AutoValue_Field.Builder().setIsRepeated(false); + return new AutoValue_Field.Builder().setIsRepeated(false).setIsMap(false); } @AutoValue.Builder @@ -45,6 +47,8 @@ public abstract static class Builder { public abstract Builder setIsRepeated(boolean isRepeated); + public abstract Builder setIsMap(boolean isMap); + public abstract Builder setResourceReference(ResourceReference resourceReference); public abstract Field build(); 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 9b7a70ddd5..275ec8d5cb 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 @@ -362,6 +362,7 @@ private static Field parseField(FieldDescriptor fieldDescriptor, Descriptor mess .setName(fieldDescriptor.getName()) .setType(TypeParser.parseType(fieldDescriptor)) .setIsRepeated(fieldDescriptor.isRepeated()) + .setIsMap(fieldDescriptor.isMapField()) .setResourceReference(resourceReference) .build(); } diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/TypeParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/TypeParser.java index 5fd58a250f..ae8439a647 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/TypeParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/TypeParser.java @@ -61,14 +61,14 @@ public class TypeParser { .build(); public static TypeNode parseType(@Nonnull FieldDescriptor field) { - if (field.isRepeated()) { - return createListType(field); - } - if (field.isMapField()) { return createMapType(field); } + if (field.isRepeated()) { + return createListType(field); + } + // Parse basic type. JavaType protoFieldType = field.getJavaType(); boolean isEnum = protoFieldType.equals(JavaType.ENUM); diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java index 67e2b8d22f..4557598770 100644 --- a/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java +++ b/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java @@ -20,6 +20,7 @@ import static junit.framework.Assert.assertTrue; import static org.junit.Assert.assertThrows; +import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.Reference; import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.VaporReference; @@ -325,6 +326,25 @@ public void parseMessages_fieldsHaveResourceReferences() { assertFalse(resourceReference.isChildType()); } + @Test + public void parseFields_mapType() { + FileDescriptor testingFileDescriptor = TestingOuterClass.getDescriptor(); + ServiceDescriptor testingService = testingFileDescriptor.getServices().get(0); + assertEquals(testingService.getName(), "Testing"); + + Map messageTypes = Parser.parseMessages(testingFileDescriptor); + Message message = messageTypes.get("Session"); + Field field = message.fieldMap().get("session_ids_to_descriptor"); + assertEquals( + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(Map.class) + .setGenerics( + Arrays.asList(TypeNode.INT_OBJECT.reference(), TypeNode.STRING.reference())) + .build()), + field.type()); + } + @Test public void parseResourceNames_inputTypeHasReferenceNotInMethodSignature() { FileDescriptor testingFileDescriptor = TestingOuterClass.getDescriptor(); diff --git a/src/test/java/com/google/api/generator/gapic/testdata/testing.proto b/src/test/java/com/google/api/generator/gapic/testdata/testing.proto index 571a4d07b0..5f8fdb2e5c 100644 --- a/src/test/java/com/google/api/generator/gapic/testdata/testing.proto +++ b/src/test/java/com/google/api/generator/gapic/testdata/testing.proto @@ -133,6 +133,8 @@ message Session { // Required. The version this session is using. Version version = 2; + + map session_ids_to_descriptor = 3; } // The request for the CreateSession method. @@ -181,8 +183,8 @@ message CreateSessionRequest { // The request for the GetSession method. message GetSessionRequest { // The session to be retrieved. - string name = 1 [ - (google.api.resource_reference).type = "showcase.googleapis.com/Session"]; + string name = 1 [(google.api.resource_reference).type = + "showcase.googleapis.com/Session"]; } // The request for the ListSessions method. @@ -207,15 +209,15 @@ message ListSessionsResponse { // Request for the DeleteSession method. message DeleteSessionRequest { // The session to be deleted. - string name = 1 [ - (google.api.resource_reference).type = "showcase.googleapis.com/Session"]; + string name = 1 [(google.api.resource_reference).type = + "showcase.googleapis.com/Session"]; } // Request message for reporting on a session. message ReportSessionRequest { // The session to be reported on. - string name = 1 [ - (google.api.resource_reference).type = "showcase.googleapis.com/Session"]; + string name = 1 [(google.api.resource_reference).type = + "showcase.googleapis.com/Session"]; } // Response message for reporting on a session. @@ -327,8 +329,8 @@ message Issue { // The request for the ListTests method. message ListTestsRequest { // The session. - string parent = 1 [ - (google.api.resource_reference).type = "showcase.googleapis.com/Session"]; + string parent = 1 [(google.api.resource_reference).type = + "showcase.googleapis.com/Session"]; // The maximum number of tests to return per page. int32 page_size = 2; @@ -352,8 +354,8 @@ message TestRun { // The name of the test. // The tests/* portion of the names are hard-coded, and do not change // from session to session. - string test = 1 [ - (google.api.resource_reference).type = "showcase.googleapis.com/Test"]; + string test = 1 + [(google.api.resource_reference).type = "showcase.googleapis.com/Test"]; // An issue found with the test run. If empty, this test run was successful. Issue issue = 2; @@ -362,14 +364,14 @@ message TestRun { // Request message for deleting a test. message DeleteTestRequest { // The test to be deleted. - string name = 1 [ - (google.api.resource_reference).type = "showcase.googleapis.com/Test"]; + string name = 1 + [(google.api.resource_reference).type = "showcase.googleapis.com/Test"]; } message VerifyTestRequest { // The test to have an answer registered to it. - string name = 1 [ - (google.api.resource_reference).type = "showcase.googleapis.com/Test"]; + string name = 1 + [(google.api.resource_reference).type = "showcase.googleapis.com/Test"]; // The answer from the test. bytes answer = 2; From bf28e84678b2d5fdb576cd4721c07487a432dd95 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 1 Sep 2020 13:53:11 -0700 Subject: [PATCH 3/3] feat: add initial batching descriptor field to ServiceStubSettings --- .../composer/BatchingDescriptorComposer.java | 241 ++++++++++++++++++ .../ServiceStubSettingsClassComposer.java | 11 + .../api/generator/gapic/composer/BUILD.bazel | 1 + .../BatchingDescriptorComposerTest.java | 230 +++++++++++++++++ .../ServiceStubSettingsClassComposerTest.java | 67 +++++ 5 files changed, 550 insertions(+) create mode 100644 src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java create mode 100644 src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java diff --git a/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java b/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java new file mode 100644 index 0000000000..3677a02b79 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java @@ -0,0 +1,241 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.api.generator.gapic.composer; + +import com.google.api.gax.batching.PartitionKey; +import com.google.api.gax.batching.RequestBuilder; +import com.google.api.gax.rpc.BatchedRequestIssuer; +import com.google.api.gax.rpc.BatchingDescriptor; +import com.google.api.generator.engine.ast.AnonymousClassExpr; +import com.google.api.generator.engine.ast.AssignmentExpr; +import com.google.api.generator.engine.ast.ConcreteReference; +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; +import com.google.api.generator.engine.ast.Reference; +import com.google.api.generator.engine.ast.ScopeNode; +import com.google.api.generator.engine.ast.TypeNode; +import com.google.api.generator.engine.ast.Variable; +import com.google.api.generator.engine.ast.VariableExpr; +import com.google.api.generator.gapic.model.GapicBatchingSettings; +import com.google.api.generator.gapic.model.Message; +import com.google.api.generator.gapic.model.Method; +import com.google.api.generator.gapic.utils.JavaStyle; +import com.google.common.base.Preconditions; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; + +public class BatchingDescriptorComposer { + private static final String BATCHING_DESC_PATTERN = "%s_BATCHING_DESC"; + + private static final Reference BATCHING_DESCRIPTOR_REF = + ConcreteReference.withClazz(BatchingDescriptor.class); + private static final Reference REQUEST_BUILDER_REF = + ConcreteReference.withClazz(RequestBuilder.class); + private static final Reference BATCHED_REQUEST_ISSUER_REF = + ConcreteReference.withClazz(BatchedRequestIssuer.class); + + private static final TypeNode PARTITION_KEY_TYPE = toType(PartitionKey.class); + + private static final String ADD_ALL_METHOD_PATTERN = "addAll%s"; + private static final String GET_LIST_METHOD_PATTERN = "get%sList"; + + public static Expr createBatchingDescriptorFieldDeclExpr( + Method method, GapicBatchingSettings batchingSettings, Map messageTypes) { + List javaMethods = new ArrayList<>(); + javaMethods.add(createGetBatchPartitionKeyMethod(method, batchingSettings, messageTypes)); + javaMethods.add(createGetRequestBuilderMethod(method, batchingSettings)); + + TypeNode batchingDescriptorType = + toType(BATCHING_DESCRIPTOR_REF, method.inputType(), method.outputType()); + AnonymousClassExpr batchingDescriptorClassExpr = + AnonymousClassExpr.builder() + .setType(batchingDescriptorType) + .setMethods(javaMethods) + .build(); + + String varName = + String.format(BATCHING_DESC_PATTERN, JavaStyle.toUpperSnakeCase(method.name())); + return AssignmentExpr.builder() + .setVariableExpr( + VariableExpr.builder() + .setVariable( + Variable.builder().setType(batchingDescriptorType).setName(varName).build()) + .setIsDecl(true) + .setScope(ScopeNode.PRIVATE) + .setIsStatic(true) + .setIsFinal(true) + .build()) + .setValueExpr(batchingDescriptorClassExpr) + .build(); + } + + private static MethodDefinition createGetBatchPartitionKeyMethod( + Method method, GapicBatchingSettings batchingSettings, Map messageTypes) { + String methodInputTypeName = method.inputType().reference().name(); + Message inputMessage = messageTypes.get(methodInputTypeName); + Preconditions.checkNotNull( + inputMessage, + String.format( + "Message %s not found for RPC method %s", methodInputTypeName, method.name())); + + VariableExpr requestVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(method.inputType()).setName("request").build()); + + List partitionKeyArgExprs = new ArrayList<>(); + for (String discriminatorFieldName : batchingSettings.discriminatorFieldNames()) { + Preconditions.checkNotNull( + inputMessage.fieldMap().get(discriminatorFieldName), + String.format( + "Batching discriminator field %s not found in message %s", + discriminatorFieldName, inputMessage.name())); + String getterMethodName = + String.format("get%s", JavaStyle.toUpperCamelCase(discriminatorFieldName)); + partitionKeyArgExprs.add( + MethodInvocationExpr.builder() + .setExprReferenceExpr(requestVarExpr) + .setMethodName(getterMethodName) + .build()); + } + Expr returnExpr = + NewObjectExpr.builder() + .setType(PARTITION_KEY_TYPE) + .setArguments(partitionKeyArgExprs) + .build(); + + return MethodDefinition.builder() + .setIsOverride(true) + .setScope(ScopeNode.PUBLIC) + .setReturnType(PARTITION_KEY_TYPE) + .setName("getBatchPartitionKey") + .setArguments(requestVarExpr.toBuilder().setIsDecl(true).build()) + .setReturnExpr(returnExpr) + .build(); + } + + private static MethodDefinition createGetRequestBuilderMethod( + Method method, GapicBatchingSettings batchingSettings) { + TypeNode builderType = toType(REQUEST_BUILDER_REF, method.inputType()); + VariableExpr builderVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(builderType).setName("builder").build()); + VariableExpr requestVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(method.inputType()).setName("request").build()); + + Expr toBuilderExpr = + AssignmentExpr.builder() + .setVariableExpr(builderVarExpr) + .setValueExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(requestVarExpr) + .setMethodName("toBuilder") + .setReturnType(builderType) + .build()) + .build(); + + String upperBatchedFieldName = JavaStyle.toUpperCamelCase(batchingSettings.batchedFieldName()); + String getFooListMethodName = String.format(GET_LIST_METHOD_PATTERN, upperBatchedFieldName); + Expr getFooListExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(requestVarExpr) + .setMethodName(getFooListMethodName) + .build(); + + String addAllMethodName = String.format(ADD_ALL_METHOD_PATTERN, upperBatchedFieldName); + Expr addAllExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(builderVarExpr) + .setMethodName(addAllMethodName) + .setArguments(getFooListExpr) + .build(); + + MethodDefinition appendRequestMethod = + MethodDefinition.builder() + .setIsOverride(true) + .setScope(ScopeNode.PUBLIC) + .setReturnType(TypeNode.VOID) + .setName("appendRequest") + .setArguments(requestVarExpr.toBuilder().setIsDecl(true).build()) + .setBody( + Arrays.asList( + IfStatement.builder() + .setConditionExpr( + MethodInvocationExpr.builder() + .setStaticReferenceType(toType(Objects.class)) + .setMethodName("isNull") + .setArguments(builderVarExpr) + .setReturnType(TypeNode.BOOLEAN) + .build()) + .setBody(Arrays.asList(ExprStatement.withExpr(toBuilderExpr))) + .setElseBody(Arrays.asList(ExprStatement.withExpr(addAllExpr))) + .build())) + .build(); + + MethodDefinition buildMethod = + MethodDefinition.builder() + .setIsOverride(true) + .setScope(ScopeNode.PUBLIC) + .setReturnType(method.inputType()) + .setName("build") + .setReturnExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(builderVarExpr) + .setMethodName("build") + .setReturnType(method.inputType()) + .build()) + .build(); + + AnonymousClassExpr requestBuilderAnonClassExpr = + AnonymousClassExpr.builder() + .setType(builderType) + .setStatements( + Arrays.asList( + ExprStatement.withExpr( + builderVarExpr + .toBuilder() + .setIsDecl(true) + .setScope(ScopeNode.PRIVATE) + .build()))) + .setMethods(Arrays.asList(appendRequestMethod, buildMethod)) + .build(); + + return MethodDefinition.builder() + .setIsOverride(true) + .setScope(ScopeNode.PUBLIC) + .setReturnType(builderType) + .setName("getRequestBuilder") + .setReturnExpr(requestBuilderAnonClassExpr) + .build(); + } + + private static TypeNode toType(Class clazz) { + return TypeNode.withReference(ConcreteReference.withClazz(clazz)); + } + + private static TypeNode toType(Reference reference, TypeNode... types) { + return TypeNode.withReference( + reference.copyAndSetGenerics( + Arrays.asList(types).stream().map(t -> t.reference()).collect(Collectors.toList()))); + } +} diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java index 737d9054b6..4b8960b16f 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java @@ -283,6 +283,17 @@ private static List createClassStatements( memberVarExprs.addAll( createPagingStaticAssignExprs(service, serviceConfig, messageTypes, types)); + + for (Method method : service.methods()) { + Optional batchingSettingOpt = + serviceConfig.getBatchingSetting(service, method); + if (batchingSettingOpt.isPresent()) { + memberVarExprs.add( + BatchingDescriptorComposer.createBatchingDescriptorFieldDeclExpr( + method, batchingSettingOpt.get(), messageTypes)); + } + } + return memberVarExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList()); } diff --git a/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel index a18b524847..740ba1b5f2 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel @@ -1,6 +1,7 @@ package(default_visibility = ["//visibility:public"]) TESTS = [ + "BatchingDescriptorComposerTest", "ComposerTest", "GrpcServiceCallableFactoryClassComposerTest", "GrpcServiceStubClassComposerTest", diff --git a/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java new file mode 100644 index 0000000000..3b5dd833ac --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java @@ -0,0 +1,230 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.api.generator.gapic.composer; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertTrue; + +import com.google.api.generator.engine.ast.Expr; +import com.google.api.generator.engine.writer.JavaWriterVisitor; +import com.google.api.generator.gapic.model.GapicBatchingSettings; +import com.google.api.generator.gapic.model.GapicServiceConfig; +import com.google.api.generator.gapic.model.Message; +import com.google.api.generator.gapic.model.Method; +import com.google.api.generator.gapic.model.ResourceName; +import com.google.api.generator.gapic.model.Service; +import com.google.api.generator.gapic.protoparser.BatchingSettingsConfigParser; +import com.google.api.generator.gapic.protoparser.Parser; +import com.google.api.generator.gapic.protoparser.ServiceConfigParser; +import com.google.logging.v2.LogEntryProto; +import com.google.logging.v2.LoggingConfigProto; +import com.google.logging.v2.LoggingMetricsProto; +import com.google.logging.v2.LoggingProto; +import com.google.protobuf.Descriptors.FileDescriptor; +import com.google.protobuf.Descriptors.ServiceDescriptor; +import com.google.pubsub.v1.PubsubProto; +import google.cloud.CommonResources; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import org.junit.Before; +import org.junit.Test; + +public class BatchingDescriptorComposerTest { + private static final String TESTFILES_DIRECTORY = + "src/test/java/com/google/api/generator/gapic/testdata/"; + + private JavaWriterVisitor writerVisitor; + + @Before + public void setUp() { + writerVisitor = new JavaWriterVisitor(); + } + + @Test + public void batchingDescriptor_hasSubresponseField() { + FileDescriptor serviceFileDescriptor = PubsubProto.getDescriptor(); + FileDescriptor commonResourcesFileDescriptor = CommonResources.getDescriptor(); + ServiceDescriptor serviceDescriptor = serviceFileDescriptor.getServices().get(0); + assertEquals("Publisher", serviceDescriptor.getName()); + + Map resourceNames = new HashMap<>(); + resourceNames.putAll(Parser.parseResourceNames(serviceFileDescriptor)); + resourceNames.putAll(Parser.parseResourceNames(commonResourcesFileDescriptor)); + + Map messageTypes = Parser.parseMessages(serviceFileDescriptor); + + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService( + serviceFileDescriptor, messageTypes, resourceNames, outputResourceNames); + + String filename = "pubsub_gapic.yaml"; + Path path = Paths.get(TESTFILES_DIRECTORY, filename); + Optional> batchingSettingsOpt = + BatchingSettingsConfigParser.parse(Optional.of(path.toString())); + assertTrue(batchingSettingsOpt.isPresent()); + + String jsonFilename = "pubsub_grpc_service_config.json"; + Path jsonPath = Paths.get(TESTFILES_DIRECTORY, jsonFilename); + Optional configOpt = + ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); + assertTrue(configOpt.isPresent()); + GapicServiceConfig config = configOpt.get(); + + Service service = services.get(0); + assertEquals("Publisher", service.name()); + Method method = findMethod(service, "Publish"); + + GapicBatchingSettings batchingSetting = batchingSettingsOpt.get().get(0); + assertEquals("Publish", batchingSetting.methodName()); + Expr batchingDescriptorExpr = + BatchingDescriptorComposer.createBatchingDescriptorFieldDeclExpr( + method, batchingSetting, messageTypes); + + batchingDescriptorExpr.accept(writerVisitor); + String expected = + createLines( + "private static final BatchingDescriptor" + + " PUBLISH_BATCHING_DESC = new BatchingDescriptor() {\n", + "@Override\n", + "public PartitionKey getBatchPartitionKey(PublishRequest request) {\n", + "return new PartitionKey(request.getTopic());\n", + "}\n", + "@Override\n", + "public RequestBuilder getRequestBuilder() {\n", + "return new RequestBuilder() {\n", + "private RequestBuilder builder;\n", + "@Override\n", + "public void appendRequest(PublishRequest request) {\n", + "if (Objects.isNull(builder)) {\n", + "builder = request.toBuilder();\n", + "} else {\n", + "builder.addAllMessages(request.getMessagesList());\n", + "}\n", + "}\n", + "@Override\n", + "public PublishRequest build() {\n", + "return builder.build();\n", + "}\n", + "};\n", + "}\n", + "}"); + assertEquals(expected, writerVisitor.write()); + } + + @Test + public void batchingDescriptor_noSubresponseField() { + FileDescriptor serviceFileDescriptor = LoggingProto.getDescriptor(); + ServiceDescriptor serviceDescriptor = serviceFileDescriptor.getServices().get(0); + assertEquals(serviceDescriptor.getName(), "LoggingServiceV2"); + + List protoFiles = + Arrays.asList( + serviceFileDescriptor, + LogEntryProto.getDescriptor(), + LoggingConfigProto.getDescriptor(), + LoggingMetricsProto.getDescriptor()); + + Map resourceNames = new HashMap<>(); + Map messageTypes = new HashMap<>(); + for (FileDescriptor fileDescriptor : protoFiles) { + resourceNames.putAll(Parser.parseResourceNames(fileDescriptor)); + messageTypes.putAll(Parser.parseMessages(fileDescriptor)); + } + + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService( + serviceFileDescriptor, messageTypes, resourceNames, outputResourceNames); + + String filename = "logging_gapic.yaml"; + Path path = Paths.get(TESTFILES_DIRECTORY, filename); + Optional> batchingSettingsOpt = + BatchingSettingsConfigParser.parse(Optional.of(path.toString())); + assertTrue(batchingSettingsOpt.isPresent()); + + String jsonFilename = "logging_grpc_service_config.json"; + Path jsonPath = Paths.get(TESTFILES_DIRECTORY, jsonFilename); + Optional configOpt = + ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); + assertTrue(configOpt.isPresent()); + GapicServiceConfig config = configOpt.get(); + + Service service = services.get(0); + assertEquals("LoggingServiceV2", service.name()); + Method method = findMethod(service, "WriteLogEntries"); + + GapicBatchingSettings batchingSetting = batchingSettingsOpt.get().get(0); + assertEquals("WriteLogEntries", batchingSetting.methodName()); + Expr batchingDescriptorExpr = + BatchingDescriptorComposer.createBatchingDescriptorFieldDeclExpr( + method, batchingSetting, messageTypes); + + batchingDescriptorExpr.accept(writerVisitor); + String expected = + createLines( + "private static final BatchingDescriptor WRITE_LOG_ENTRIES_BATCHING_DESC = new" + + " BatchingDescriptor() {\n", + "@Override\n", + "public PartitionKey getBatchPartitionKey(WriteLogEntriesRequest request) {\n", + "return new PartitionKey(request.getLogName(), request.getResource()," + + " request.getLabels());\n", + "}\n", + "@Override\n", + "public RequestBuilder getRequestBuilder() {\n", + "return new RequestBuilder() {\n", + "private RequestBuilder builder;\n", + "@Override\n", + "public void appendRequest(WriteLogEntriesRequest request) {\n", + "if (Objects.isNull(builder)) {\n", + "builder = request.toBuilder();\n", + "} else {\n", + "builder.addAllEntries(request.getEntriesList());\n", + "}\n", + "}\n", + "@Override\n", + "public WriteLogEntriesRequest build() {\n", + "return builder.build();\n", + "}\n", + "};\n", + "}\n", + "}"); + + assertEquals(expected, writerVisitor.write()); + } + + private static Method findMethod(Service service, String methodName) { + for (Method m : service.methods()) { + if (m.name().equals(methodName)) { + return m; + } + } + return null; + } + + private static String createLines(String... lines) { + // Cast to get rid of warnings. + return String.format(new String(new char[lines.length]).replace("\0", "%s"), (Object[]) lines); + } +} diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java index 4ef13a2350..72ff6e2e6e 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java @@ -684,6 +684,8 @@ private static List parseServices( + "import com.google.api.gax.batching.BatchingSettings;\n" + "import com.google.api.gax.batching.FlowControlSettings;\n" + "import com.google.api.gax.batching.FlowController;\n" + + "import com.google.api.gax.batching.PartitionKey;\n" + + "import com.google.api.gax.batching.RequestBuilder;\n" + "import com.google.api.gax.core.GaxProperties;\n" + "import com.google.api.gax.core.GoogleCredentialsProvider;\n" + "import com.google.api.gax.core.InstantiatingExecutorProvider;\n" @@ -960,6 +962,40 @@ private static List parseServices( + " futureResponse);\n" + " }\n" + " };\n" + + " private static final BatchingDescriptor\n" + + " WRITE_LOG_ENTRIES_BATCHING_DESC =\n" + + " new BatchingDescriptor()" + + " {\n" + + " @Override\n" + + " public PartitionKey getBatchPartitionKey(WriteLogEntriesRequest request)" + + " {\n" + + " return new PartitionKey(\n" + + " request.getLogName(), request.getResource()," + + " request.getLabels());\n" + + " }\n" + + "\n" + + " @Override\n" + + " public RequestBuilder getRequestBuilder() {\n" + + " return new RequestBuilder() {\n" + + " private RequestBuilder builder;\n" + + "\n" + + " @Override\n" + + " public void appendRequest(WriteLogEntriesRequest request) {\n" + + " if (Objects.isNull(builder)) {\n" + + " builder = request.toBuilder();\n" + + " } else {\n" + + " builder.addAllEntries(request.getEntriesList());\n" + + " }\n" + + " }\n" + + "\n" + + " @Override\n" + + " public WriteLogEntriesRequest build() {\n" + + " return builder.build();\n" + + " }\n" + + " };\n" + + " }\n" + + " };\n" + "\n" + " public UnaryCallSettings deleteLogSettings() {\n" + " return deleteLogSettings;\n" @@ -1294,6 +1330,8 @@ private static List parseServices( + "import com.google.api.gax.batching.BatchingSettings;\n" + "import com.google.api.gax.batching.FlowControlSettings;\n" + "import com.google.api.gax.batching.FlowController;\n" + + "import com.google.api.gax.batching.PartitionKey;\n" + + "import com.google.api.gax.batching.RequestBuilder;\n" + "import com.google.api.gax.core.GaxProperties;\n" + "import com.google.api.gax.core.GoogleCredentialsProvider;\n" + "import com.google.api.gax.core.InstantiatingExecutorProvider;\n" @@ -1569,6 +1607,35 @@ private static List parseServices( + " futureResponse);\n" + " }\n" + " };\n" + + " private static final BatchingDescriptor" + + " PUBLISH_BATCHING_DESC =\n" + + " new BatchingDescriptor() {\n" + + " @Override\n" + + " public PartitionKey getBatchPartitionKey(PublishRequest request) {\n" + + " return new PartitionKey(request.getTopic());\n" + + " }\n" + + "\n" + + " @Override\n" + + " public RequestBuilder getRequestBuilder() {\n" + + " return new RequestBuilder() {\n" + + " private RequestBuilder builder;\n" + + "\n" + + " @Override\n" + + " public void appendRequest(PublishRequest request) {\n" + + " if (Objects.isNull(builder)) {\n" + + " builder = request.toBuilder();\n" + + " } else {\n" + + " builder.addAllMessages(request.getMessagesList());\n" + + " }\n" + + " }\n" + + "\n" + + " @Override\n" + + " public PublishRequest build() {\n" + + " return builder.build();\n" + + " }\n" + + " };\n" + + " }\n" + + " };\n" + "\n" + " public UnaryCallSettings createTopicSettings() {\n" + " return createTopicSettings;\n"