From 3dffa8d11e05d5248fd499bfd21bdee20f37fae0 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Mon, 31 Aug 2020 13:55:43 -0700 Subject: [PATCH 01/15] 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 02/15] 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 03/15] 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" From 902fd41d01ded42702cee04c82ba8fff51e341d5 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 1 Sep 2020 15:55:10 -0700 Subject: [PATCH 04/15] feat: support '? extends Foo' wildcard bounded references --- .../engine/ast/ConcreteReference.java | 43 ++++++++++++++++-- .../api/generator/engine/ast/Reference.java | 7 ++- .../api/generator/engine/ast/TypeNode.java | 20 +++++++-- .../generator/engine/ast/VaporReference.java | 11 +++++ .../engine/writer/ImportWriterVisitor.java | 13 ++++-- .../engine/ast/ConcreteReferenceTest.java | 8 ++++ .../generator/engine/ast/TypeNodeTest.java | 29 ++++++++++++ .../writer/ImportWriterVisitorTest.java | 44 +++++++++++++++++++ .../engine/writer/JavaWriterVisitorTest.java | 37 ++++++++++++++++ 9 files changed, 202 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java b/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java index 144820c7ab..c95823fc13 100644 --- a/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java +++ b/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java @@ -17,18 +17,29 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; import java.util.List; +import java.util.Objects; +import javax.annotation.Nullable; @AutoValue public abstract class ConcreteReference implements Reference { + private static final String EXTENDS = "extends"; + private static final String COMMA = ", "; private static final String DOT = "."; + private static final String SPACE = " "; private static final String LEFT_ANGLE = "<"; private static final String RIGHT_ANGLE = ">"; private static final String QUESTION_MARK = "?"; + private static final Class WILDCARD_CLAZZ = ReferenceWildcard.class; + // Private. abstract Class clazz(); + @Nullable + @Override + public abstract Reference wildcardUpperBound(); + @Override public abstract ImmutableList generics(); @@ -38,8 +49,15 @@ public abstract class ConcreteReference implements Reference { @Override public String name() { StringBuilder sb = new StringBuilder(); - if (this.equals(TypeNode.WILDCARD_REFERENCE)) { + if (isWildcard()) { sb.append(QUESTION_MARK); + if (wildcardUpperBound() != null) { + // Handle the upper bound. + sb.append(SPACE); + sb.append(EXTENDS); + sb.append(SPACE); + sb.append(wildcardUpperBound().name()); + } } else { if (hasEnclosingClass() && !isStaticImport()) { sb.append(clazz().getEnclosingClass().getSimpleName()); @@ -117,6 +135,11 @@ public boolean isAssignableFrom(Reference other) { return clazz().isAssignableFrom(((ConcreteReference) other).clazz()); } + @Override + public boolean isWildcard() { + return clazz().equals(WILDCARD_CLAZZ); + } + @Override public boolean equals(Object o) { if (!(o instanceof ConcreteReference)) { @@ -124,12 +147,16 @@ public boolean equals(Object o) { } ConcreteReference ref = (ConcreteReference) o; - return clazz().equals(ref.clazz()) && generics().equals(ref.generics()); + return clazz().equals(ref.clazz()) + && generics().equals(ref.generics()) + && Objects.equals(wildcardUpperBound(), ref.wildcardUpperBound()); } @Override public int hashCode() { - return 17 * clazz().hashCode() + 31 * generics().hashCode(); + int wildcardUpperBoundHash = + wildcardUpperBound() == null ? 0 : 11 * wildcardUpperBound().hashCode(); + return 17 * clazz().hashCode() + 31 * generics().hashCode() + wildcardUpperBoundHash; } @Override @@ -141,6 +168,14 @@ public static ConcreteReference withClazz(Class clazz) { return builder().setClazz(clazz).build(); } + public static ConcreteReference wildcard() { + return withClazz(ReferenceWildcard.class); + } + + public static ConcreteReference wildcardWithUpperBound(Reference upperBoundReference) { + return builder().setClazz(WILDCARD_CLAZZ).setWildcardUpperBound(upperBoundReference).build(); + } + public static Builder builder() { return new AutoValue_ConcreteReference.Builder() .setGenerics(ImmutableList.of()) @@ -154,6 +189,8 @@ public static Builder builder() { public abstract static class Builder { public abstract Builder setClazz(Class clazz); + public abstract Builder setWildcardUpperBound(Reference reference); + public abstract Builder setGenerics(List clazzes); public abstract Builder setIsStaticImport(boolean isStaticImport); diff --git a/src/main/java/com/google/api/generator/engine/ast/Reference.java b/src/main/java/com/google/api/generator/engine/ast/Reference.java index f0e92ef3eb..6fd856c78f 100644 --- a/src/main/java/com/google/api/generator/engine/ast/Reference.java +++ b/src/main/java/com/google/api/generator/engine/ast/Reference.java @@ -30,6 +30,11 @@ public interface Reference { @Nullable String enclosingClassName(); + @Nullable + Reference wildcardUpperBound(); + + Reference copyAndSetGenerics(List generics); + // Valid only for nested classes. boolean isStaticImport(); @@ -42,5 +47,5 @@ public interface Reference { boolean isAssignableFrom(Reference other); - Reference copyAndSetGenerics(List generics); + boolean isWildcard(); } diff --git a/src/main/java/com/google/api/generator/engine/ast/TypeNode.java b/src/main/java/com/google/api/generator/engine/ast/TypeNode.java index d1bb85c552..65e5f16130 100644 --- a/src/main/java/com/google/api/generator/engine/ast/TypeNode.java +++ b/src/main/java/com/google/api/generator/engine/ast/TypeNode.java @@ -25,8 +25,7 @@ @AutoValue public abstract class TypeNode implements AstNode { static final Reference EXCEPTION_REFERENCE = ConcreteReference.withClazz(Exception.class); - public static final Reference WILDCARD_REFERENCE = - ConcreteReference.withClazz(ReferenceWildcard.class); + public static final Reference WILDCARD_REFERENCE = ConcreteReference.wildcard(); public enum TypeKind { BYTE, @@ -101,7 +100,22 @@ public abstract static class Builder { public abstract Builder setReference(Reference reference); - public abstract TypeNode build(); + // Private. + abstract Reference reference(); + + abstract TypeNode autoBuild(); + + public TypeNode build() { + if (reference() != null) { + // Disallow top-level wildcard references. + Preconditions.checkState( + !reference().isWildcard(), + String.format( + "The top-level referenece in a type cannot be a wildcard, found %s", + reference().name())); + } + return autoBuild(); + } } // TODO(miraleung): More type creation helpers to come... diff --git a/src/main/java/com/google/api/generator/engine/ast/VaporReference.java b/src/main/java/com/google/api/generator/engine/ast/VaporReference.java index 736c41daff..4ba17aa356 100644 --- a/src/main/java/com/google/api/generator/engine/ast/VaporReference.java +++ b/src/main/java/com/google/api/generator/engine/ast/VaporReference.java @@ -41,6 +41,12 @@ public abstract class VaporReference implements Reference { @Override public abstract String enclosingClassName(); + @Nullable + @Override + public Reference wildcardUpperBound() { + return null; + } + @Override public String fullName() { // TODO(unsupported): Nested classes with depth greater than 1. @@ -75,6 +81,11 @@ public boolean isAssignableFrom(Reference other) { return false; } + @Override + public boolean isWildcard() { + return false; + } + abstract String plainName(); @Override diff --git a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java index f8b16a57b3..dfdadbb307 100644 --- a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java @@ -50,6 +50,7 @@ import com.google.api.generator.engine.ast.WhileStatement; 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.Set; @@ -340,9 +341,15 @@ private void variableExpressions(List expressions) { private void references(List refs) { for (Reference ref : refs) { // Don't need to import this. - if ((!ref.isStaticImport() - && (ref.isFromPackage(PKG_JAVA_LANG) || ref.isFromPackage(currentPackage))) - || ref.equals(TypeNode.WILDCARD_REFERENCE)) { + if (!ref.isStaticImport() + && (ref.isFromPackage(PKG_JAVA_LANG) || ref.isFromPackage(currentPackage))) { + continue; + } + + if (ref.isWildcard()) { + if (ref.wildcardUpperBound() != null) { + references(Arrays.asList(ref.wildcardUpperBound())); + } continue; } diff --git a/src/test/java/com/google/api/generator/engine/ast/ConcreteReferenceTest.java b/src/test/java/com/google/api/generator/engine/ast/ConcreteReferenceTest.java index 717f229822..b284b1bb3a 100644 --- a/src/test/java/com/google/api/generator/engine/ast/ConcreteReferenceTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/ConcreteReferenceTest.java @@ -140,4 +140,12 @@ public void isSupertype_nestedGenerics() { assertTrue(typeOne.isSupertypeOrEquals(typeTwo)); assertFalse(typeTwo.isSupertypeOrEquals(typeOne)); } + + @Test + public void wildcards() { + assertEquals("?", ConcreteReference.wildcard().name()); + assertEquals( + "? extends String", + ConcreteReference.wildcardWithUpperBound(TypeNode.STRING.reference()).name()); + } } diff --git a/src/test/java/com/google/api/generator/engine/ast/TypeNodeTest.java b/src/test/java/com/google/api/generator/engine/ast/TypeNodeTest.java index 3cd985a119..3a446f15d9 100644 --- a/src/test/java/com/google/api/generator/engine/ast/TypeNodeTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/TypeNodeTest.java @@ -16,6 +16,7 @@ import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertTrue; +import static org.junit.Assert.assertThrows; import com.google.api.generator.engine.ast.TypeNode.TypeKind; import java.util.Arrays; @@ -88,4 +89,32 @@ public void equals_basic() { assertFalse(TypeNode.CHAR.equals(TypeNode.NULL)); assertFalse(INTEGER_ARRAY.equals(INT_ARRAY)); } + + @Test + public void type_wildcardGenerics() { + // No exception thrown equates to success. + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(List.class) + .setGenerics(Arrays.asList(ConcreteReference.wildcard())) + .build()); + } + + @Test + public void type_wildcardUpperBoundGenerics() { + // No exception thrown equates to success. + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(List.class) + .setGenerics( + Arrays.asList( + ConcreteReference.wildcardWithUpperBound(TypeNode.STRING.reference()))) + .build()); + } + + @Test + public void invalidType_topLevelWildcard() { + assertThrows( + IllegalStateException.class, () -> TypeNode.withReference(ConcreteReference.wildcard())); + } } diff --git a/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java index 815588b8b4..c7e023bc26 100644 --- a/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java @@ -473,6 +473,50 @@ public void writeVariableExprImports_staticReference() { "import com.google.api.generator.engine.ast.TypeNode;\n\n")); } + @Test + public void writeVariableExprImports_wildcardType() { + TypeNode wildcardListType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(List.class) + .setGenerics(Arrays.asList(TypeNode.WILDCARD_REFERENCE)) + .build()); + + // Constructs `List x`. + Variable variable = Variable.builder().setName("x").setType(wildcardListType).build(); + VariableExpr variableExpr = + VariableExpr.builder().setIsDecl(true).setVariable(variable).build(); + + variableExpr.accept(writerVisitor); + assertEquals(writerVisitor.write(), "import java.util.List;\n\n"); + } + + @Test + public void writeVariableExprImport_wildcardTypeWithUpperBound() { + TypeNode wildcardListType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(List.class) + .setGenerics( + Arrays.asList( + ConcreteReference.wildcardWithUpperBound( + ConcreteReference.withClazz(Expr.class)))) + .build()); + + // Constructs `List x`. + Variable variable = Variable.builder().setName("x").setType(wildcardListType).build(); + VariableExpr variableExpr = + VariableExpr.builder().setIsDecl(true).setVariable(variable).build(); + + variableExpr.accept(writerVisitor); + assertEquals( + writerVisitor.write(), + String.format( + createLines(2), + "import com.google.api.generator.engine.ast.Expr;\n", + "import java.util.List;\n\n")); + } + @Test public void writeVariableExprImports_reference() { Variable variable = diff --git a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java index 9b38d81392..a3e2705bea 100644 --- a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java @@ -204,6 +204,43 @@ public void writeVariableExpr_basic() { assertEquals(writerVisitor.write(), "x"); } + @Test + public void writeVariableExpr_wildcardType() { + TypeNode wildcardListType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(List.class) + .setGenerics(Arrays.asList(TypeNode.WILDCARD_REFERENCE)) + .build()); + + Variable variable = Variable.builder().setName("x").setType(wildcardListType).build(); + VariableExpr variableExpr = + VariableExpr.builder().setIsDecl(true).setVariable(variable).build(); + + variableExpr.accept(writerVisitor); + assertEquals(writerVisitor.write(), "List x"); + } + + @Test + public void writeVariableExpr_wildcardTypeWithUpperBound() { + TypeNode wildcardListType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(List.class) + .setGenerics( + Arrays.asList( + ConcreteReference.wildcardWithUpperBound( + ConcreteReference.withClazz(Expr.class)))) + .build()); + + Variable variable = Variable.builder().setName("x").setType(wildcardListType).build(); + VariableExpr variableExpr = + VariableExpr.builder().setIsDecl(true).setVariable(variable).build(); + + variableExpr.accept(writerVisitor); + assertEquals(writerVisitor.write(), "List x"); + } + @Test public void writeVariableExpr_staticReference() { VariableExpr variableExpr = From 8efb572b7dd3ef40f1b1e57b9198699e734643a8 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 1 Sep 2020 15:56:22 -0700 Subject: [PATCH 05/15] fix: speed up precommit checks with --disk_cache --- .githooks/pre-commit | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 06a7a3bf7b..d8091b4316 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -40,10 +40,10 @@ function echo_success { function header_check_preparation { echo_status "Setting up license check environment" export GOPATH=$(go env GOPATH) - if [ $? -ne 0 ]; - then - echo_status "Please install Go first, instructions can be found here: https://golang.org/doc/install." - else + if [ $? -ne 0 ]; + then + echo_status "Please install Go first, instructions can be found here: https://golang.org/doc/install." + else export ENV_PATH=$(echo $PATH) if [[ $ENV_PATH != *$GOPATH* ]]; then @@ -51,7 +51,7 @@ function header_check_preparation { export PATH=$GOPATH/bin:$PATH fi which addlicense - if [ $? -ne 0 ]; + if [ $? -ne 0 ]; then echo_status "addlicense tool is not yet installed, downloading it now." go get -u github.com/google/addlicense @@ -59,6 +59,13 @@ function header_check_preparation { fi } +# Disk cache. +BAZEL_CACHE_DIR=/tmp/bazel_cache_gapic_generator_java +if [ ! -d $BAZEL_CACHE_DIR ] +then + mkdir $BAZEL_CACHE_DIR +fi + # Constants. # Check only the staged files. NUM_TOTAL_FILES_CHANGED=$(git diff --cached --name-only | wc -l) @@ -80,7 +87,7 @@ fi if [ $NUM_JAVA_FILES_CHANGED -gt 0 ] then echo_status "Running Java linter..." - bazel build //:google_java_format_verification + bazel build --disk_cache="$BAZEL_CACHE_DIR" //:google_java_format_verification FORMAT_STATUS=$? if [ $FORMAT_STATUS != 0 ] then @@ -93,7 +100,7 @@ fi if [ $NUM_JAVA_FILES_CHANGED -gt 0 ] || [ $NUM_BAZEL_FILES_CHANGED -gt 0 ] then echo_status "Checking the build..." - bazel build //... + bazel build --disk_cache="$BAZEL_CACHE_DIR" //... BUILD_STATUS=$? if [ $BUILD_STATUS != 0 ] then @@ -105,7 +112,7 @@ fi if [ $NUM_JAVA_FILES_CHANGED -gt 0 ] then echo_status "Checking tests..." - bazel test //... + bazel test --disk_cache="$BAZEL_CACHE_DIR" //... TEST_STATUS=$? if [ $TEST_STATUS != 0 ] then From 66a7e1c5643ba359158ed205e8849ca3e1271808 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 1 Sep 2020 15:57:08 -0700 Subject: [PATCH 06/15] feat: add splitException, count{Elements,Bytes} batching descriptor methods to ServiceStubSettings --- .../composer/BatchingDescriptorComposer.java | 100 ++++++++++++++++++ .../BatchingDescriptorComposerTest.java | 32 ++++++ .../ServiceStubSettingsClassComposerTest.java | 44 ++++++++ 3 files changed, 176 insertions(+) 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 index 3677a02b79..bcdb809df3 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java @@ -23,6 +23,7 @@ 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.ForStatement; import com.google.api.generator.engine.ast.IfStatement; import com.google.api.generator.engine.ast.MethodDefinition; import com.google.api.generator.engine.ast.MethodInvocationExpr; @@ -39,6 +40,7 @@ import com.google.common.base.Preconditions; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Objects; @@ -58,6 +60,7 @@ public class BatchingDescriptorComposer { private static final String ADD_ALL_METHOD_PATTERN = "addAll%s"; private static final String GET_LIST_METHOD_PATTERN = "get%sList"; + private static final String GET_COUNT_METHOD_PATTERN = "get%sCount"; public static Expr createBatchingDescriptorFieldDeclExpr( Method method, GapicBatchingSettings batchingSettings, Map messageTypes) { @@ -65,6 +68,10 @@ public static Expr createBatchingDescriptorFieldDeclExpr( javaMethods.add(createGetBatchPartitionKeyMethod(method, batchingSettings, messageTypes)); javaMethods.add(createGetRequestBuilderMethod(method, batchingSettings)); + javaMethods.add(createSplitExceptionMethod(method)); + javaMethods.add(createCountElementsMethod(method, batchingSettings)); + javaMethods.add(createCountByteSMethod(method)); + TypeNode batchingDescriptorType = toType(BATCHING_DESCRIPTOR_REF, method.inputType(), method.outputType()); AnonymousClassExpr batchingDescriptorClassExpr = @@ -229,6 +236,99 @@ private static MethodDefinition createGetRequestBuilderMethod( .build(); } + private static MethodDefinition createSplitExceptionMethod(Method method) { + VariableExpr throwableVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(toType(Throwable.class)).setName("throwable").build()); + + TypeNode batchedRequestIssuerType = toType(BATCHED_REQUEST_ISSUER_REF, method.outputType()); + TypeNode batchVarType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(Collection.class) + .setGenerics( + Arrays.asList( + ConcreteReference.wildcardWithUpperBound( + batchedRequestIssuerType.reference()))) + .build()); + VariableExpr batchVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(batchVarType).setName("batch").build()); + VariableExpr responderVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(batchedRequestIssuerType).setName("responder").build()); + + ForStatement forStatement = + ForStatement.builder() + .setLocalVariableExpr(responderVarExpr.toBuilder().setIsDecl(true).build()) + .setCollectionExpr(batchVarExpr) + .setBody( + Arrays.asList( + ExprStatement.withExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(responderVarExpr) + .setMethodName("setException") + .setArguments(throwableVarExpr) + .build()))) + .build(); + + return MethodDefinition.builder() + .setIsOverride(true) + .setScope(ScopeNode.PUBLIC) + .setReturnType(TypeNode.VOID) + .setName("splitException") + .setArguments( + Arrays.asList(throwableVarExpr, batchVarExpr).stream() + .map(v -> v.toBuilder().setIsDecl(true).build()) + .collect(Collectors.toList())) + .setBody(Arrays.asList(forStatement)) + .build(); + } + + private static MethodDefinition createCountElementsMethod( + Method method, GapicBatchingSettings batchingSettings) { + String getFooCountMethodName = + String.format( + GET_COUNT_METHOD_PATTERN, + JavaStyle.toUpperCamelCase(batchingSettings.batchedFieldName())); + VariableExpr requestVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(method.inputType()).setName("request").build()); + + return MethodDefinition.builder() + .setIsOverride(true) + .setScope(ScopeNode.PUBLIC) + .setReturnType(TypeNode.LONG) + .setName("countElements") + .setArguments(requestVarExpr.toBuilder().setIsDecl(true).build()) + .setReturnExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(requestVarExpr) + .setMethodName(getFooCountMethodName) + .setReturnType(TypeNode.LONG) + .build()) + .build(); + } + + private static MethodDefinition createCountByteSMethod(Method method) { + VariableExpr requestVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(method.inputType()).setName("request").build()); + return MethodDefinition.builder() + .setIsOverride(true) + .setScope(ScopeNode.PUBLIC) + .setReturnType(TypeNode.LONG) + .setName("countBytes") + .setArguments(requestVarExpr.toBuilder().setIsDecl(true).build()) + .setReturnExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(requestVarExpr) + .setMethodName("getSerializedSize") + .setReturnType(TypeNode.LONG) + .build()) + .build(); + } + private static TypeNode toType(Class clazz) { return TypeNode.withReference(ConcreteReference.withClazz(clazz)); } 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 index 3b5dd833ac..e412e0a832 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java @@ -128,6 +128,22 @@ public void batchingDescriptor_hasSubresponseField() { "}\n", "};\n", "}\n", + "@Override\n", + "public void splitException(", + "Throwable throwable, ", + "Collection> batch) {\n", + "for (BatchedRequestIssuer responder : batch) {\n", + "responder.setException(throwable);\n", + "}\n", + "}\n", + "@Override\n", + "public long countElements(PublishRequest request) {\n", + "return request.getMessagesCount();\n", + "}\n", + "@Override\n", + "public long countBytes(PublishRequest request) {\n", + "return request.getSerializedSize();\n", + "}\n", "}"); assertEquals(expected, writerVisitor.write()); } @@ -209,6 +225,22 @@ public void batchingDescriptor_noSubresponseField() { "}\n", "};\n", "}\n", + "@Override\n", + "public void splitException(", + "Throwable throwable, ", + "Collection> batch) {\n", + "for (BatchedRequestIssuer responder : batch) {\n", + "responder.setException(throwable);\n", + "}\n", + "}\n", + "@Override\n", + "public long countElements(WriteLogEntriesRequest request) {\n", + "return request.getEntriesCount();\n", + "}\n", + "@Override\n", + "public long countBytes(WriteLogEntriesRequest request) {\n", + "return request.getSerializedSize();\n", + "}\n", "}"); assertEquals(expected, writerVisitor.write()); 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 72ff6e2e6e..10456faed3 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 @@ -695,6 +695,7 @@ private static List parseServices( + "import com.google.api.gax.retrying.RetrySettings;\n" + "import com.google.api.gax.rpc.ApiCallContext;\n" + "import com.google.api.gax.rpc.ApiClientHeaderProvider;\n" + + "import com.google.api.gax.rpc.BatchedRequestIssuer;\n" + "import com.google.api.gax.rpc.BatchingCallSettings;\n" + "import com.google.api.gax.rpc.BatchingDescriptor;\n" + "import com.google.api.gax.rpc.ClientContext;\n" @@ -723,6 +724,7 @@ private static List parseServices( + "import com.google.logging.v2.WriteLogEntriesResponse;\n" + "import com.google.protobuf.Empty;\n" + "import java.io.IOException;\n" + + "import java.util.Collection;\n" + "import java.util.List;\n" + "import java.util.Objects;\n" + "import javax.annotation.Generated;\n" @@ -995,6 +997,27 @@ private static List parseServices( + " }\n" + " };\n" + " }\n" + + "\n" + + " @Override\n" + + " public void splitException(\n" + + " Throwable throwable,\n" + + " Collection>" + + " batch) {\n" + + " for (BatchedRequestIssuer responder : batch)" + + " {\n" + + " responder.setException(throwable);\n" + + " }\n" + + " }\n" + + "\n" + + " @Override\n" + + " public long countElements(WriteLogEntriesRequest request) {\n" + + " return request.getEntriesCount();\n" + + " }\n" + + "\n" + + " @Override\n" + + " public long countBytes(WriteLogEntriesRequest request) {\n" + + " return request.getSerializedSize();\n" + + " }\n" + " };\n" + "\n" + " public UnaryCallSettings deleteLogSettings() {\n" @@ -1341,6 +1364,7 @@ private static List parseServices( + "import com.google.api.gax.retrying.RetrySettings;\n" + "import com.google.api.gax.rpc.ApiCallContext;\n" + "import com.google.api.gax.rpc.ApiClientHeaderProvider;\n" + + "import com.google.api.gax.rpc.BatchedRequestIssuer;\n" + "import com.google.api.gax.rpc.BatchingCallSettings;\n" + "import com.google.api.gax.rpc.BatchingDescriptor;\n" + "import com.google.api.gax.rpc.ClientContext;\n" @@ -1373,6 +1397,7 @@ private static List parseServices( + "import com.google.pubsub.v1.Topic;\n" + "import com.google.pubsub.v1.UpdateTopicRequest;\n" + "import java.io.IOException;\n" + + "import java.util.Collection;\n" + "import java.util.List;\n" + "import java.util.Objects;\n" + "import javax.annotation.Generated;\n" @@ -1635,6 +1660,25 @@ private static List parseServices( + " }\n" + " };\n" + " }\n" + + "\n" + + " @Override\n" + + " public void splitException(\n" + + " Throwable throwable,\n" + + " Collection> batch) {\n" + + " for (BatchedRequestIssuer responder : batch) {\n" + + " responder.setException(throwable);\n" + + " }\n" + + " }\n" + + "\n" + + " @Override\n" + + " public long countElements(PublishRequest request) {\n" + + " return request.getMessagesCount();\n" + + " }\n" + + "\n" + + " @Override\n" + + " public long countBytes(PublishRequest request) {\n" + + " return request.getSerializedSize();\n" + + " }\n" + " };\n" + "\n" + " public UnaryCallSettings createTopicSettings() {\n" From 1a5c8f81a8b0df01ed9d199106a92e2ac6f05a3d Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 1 Sep 2020 16:59:41 -0700 Subject: [PATCH 07/15] feat: add GeneralForStatement --- .../generator/engine/ast/AstNodeVisitor.java | 2 + .../engine/ast/GeneralForStatement.java | 103 ++++++++++++++++++ .../engine/writer/ImportWriterVisitor.java | 9 ++ .../engine/writer/JavaWriterVisitor.java | 31 ++++++ .../api/generator/engine/ast/BUILD.bazel | 1 + .../engine/ast/GeneralForStatementTest.java | 88 +++++++++++++++ .../engine/writer/JavaWriterVisitorTest.java | 21 ++++ 7 files changed, 255 insertions(+) create mode 100644 src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java create mode 100644 src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java diff --git a/src/main/java/com/google/api/generator/engine/ast/AstNodeVisitor.java b/src/main/java/com/google/api/generator/engine/ast/AstNodeVisitor.java index 0be36c21a7..52276d949a 100644 --- a/src/main/java/com/google/api/generator/engine/ast/AstNodeVisitor.java +++ b/src/main/java/com/google/api/generator/engine/ast/AstNodeVisitor.java @@ -67,6 +67,8 @@ public interface AstNodeVisitor { public void visit(ForStatement forStatement); + public void visit(GeneralForStatement generalForStatement); + public void visit(WhileStatement whileStatement); public void visit(TryCatchStatement tryCatchStatement); diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java new file mode 100644 index 0000000000..fba06d97d1 --- /dev/null +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -0,0 +1,103 @@ +// 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.engine.ast; + +import com.google.auto.value.AutoValue; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; +import java.util.Collections; +import java.util.List; + +@AutoValue +public abstract class GeneralForStatement implements Statement { + public abstract Expr initializationExpr(); + + // TODO(summerji): Integrate OperationExpr here. Start by uncommenting the following section to + // replace the localVariableExpr and maxSizeExpr getters after it. + /* + // Uses the same terminology as https://docs.oracle.com/javase/tutorial/java/nutsandbolts/for.html. + public abstract Expr terminationExpr(); + public abstract Expr incrementExpr(); + */ + + public abstract VariableExpr localVariableExpr(); + + public abstract Expr maxSizeExpr(); + + public abstract ImmutableList body(); + + @Override + public void accept(AstNodeVisitor visitor) { + visitor.visit(this); + } + + // Convenience wrapper. + public static GeneralForStatement incrementWith( + VariableExpr variableExpr, Expr maxSizeExpr, List body) { + // TODO(summerji): Do some integration here, in JavaWriterVisitor, in ImportWriterVisitor, and + // add more tests. + return builder() + .setLocalVariableExpr(variableExpr.toBuilder().setIsDecl(false).build()) + .setMaxSizeExpr(maxSizeExpr) + .setBody(body) + .build(); + } + + public static Builder builder() { + return new AutoValue_GeneralForStatement.Builder().setBody(Collections.emptyList()); + } + + @AutoValue.Builder + public abstract static class Builder { + public abstract Builder setInitializationExpr(Expr initializationExpr); + + public abstract Builder setBody(List body); + + // Private. + abstract Builder setLocalVariableExpr(VariableExpr variableExpr); + + abstract Builder setMaxSizeExpr(Expr maxSizeExpr); + + abstract VariableExpr localVariableExpr(); + + abstract GeneralForStatement autoBuild(); + + // Type-checking will be done in the sub-expressions. + public GeneralForStatement build() { + VariableExpr varExpr = localVariableExpr(); + Preconditions.checkState( + varExpr.scope().equals(ScopeNode.LOCAL), + String.format( + "Variable %s in a general for-loop cannot have a non-local scope", + varExpr.variable().identifier().name())); + Preconditions.checkState( + !varExpr.isStatic() && !varExpr.isFinal(), + String.format( + "Variable %s in a general for-loop cannot be static or final", + varExpr.variable().identifier().name())); + setInitializationExpr( + AssignmentExpr.builder() + .setVariableExpr(varExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr( + ValueExpr.withValue( + PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build())) + .build()); + // TODO(summerji): Remove the following two lines. + // This temporary workaround will be removed soon, so it doesn't need a test. + setLocalVariableExpr(varExpr.toBuilder().setIsDecl(false).build()); + return autoBuild(); + } + } +} diff --git a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java index dfdadbb307..47a61bc3af 100644 --- a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java @@ -27,6 +27,7 @@ import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; import com.google.api.generator.engine.ast.ForStatement; +import com.google.api.generator.engine.ast.GeneralForStatement; import com.google.api.generator.engine.ast.IdentifierNode; import com.google.api.generator.engine.ast.IfStatement; import com.google.api.generator.engine.ast.InstanceofExpr; @@ -241,6 +242,14 @@ public void visit(ForStatement forStatement) { statements(forStatement.body()); } + @Override + public void visit(GeneralForStatement generalForStatement) { + generalForStatement.initializationExpr().accept(this); + generalForStatement.localVariableExpr().accept(this); + generalForStatement.maxSizeExpr().accept(this); + statements(generalForStatement.body()); + } + @Override public void visit(WhileStatement whileStatement) { whileStatement.conditionExpr().accept(this); diff --git a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java index b246e7234f..f9ee6141e6 100644 --- a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java @@ -27,6 +27,7 @@ import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; import com.google.api.generator.engine.ast.ForStatement; +import com.google.api.generator.engine.ast.GeneralForStatement; import com.google.api.generator.engine.ast.IdentifierNode; import com.google.api.generator.engine.ast.IfStatement; import com.google.api.generator.engine.ast.InstanceofExpr; @@ -461,6 +462,36 @@ public void visit(ForStatement forStatement) { newline(); } + @Override + public void visit(GeneralForStatement generalForStatement) { + buffer.append(FOR); + space(); + leftParen(); + generalForStatement.initializationExpr().accept(this); + semicolon(); + space(); + + generalForStatement.localVariableExpr().accept(this); + space(); + buffer.append(LEFT_ANGLE); + space(); + generalForStatement.maxSizeExpr().accept(this); + semicolon(); + space(); + + generalForStatement.localVariableExpr().accept(this); + // TODO(summerji): Remove the following temporary workaround. + buffer.append("++"); + rightParen(); + space(); + leftBrace(); + newline(); + + statements(generalForStatement.body()); + rightBrace(); + newline(); + } + @Override public void visit(WhileStatement whileStatement) { buffer.append(WHILE); diff --git a/src/test/java/com/google/api/generator/engine/ast/BUILD.bazel b/src/test/java/com/google/api/generator/engine/ast/BUILD.bazel index bfeff4498a..c2cf286570 100644 --- a/src/test/java/com/google/api/generator/engine/ast/BUILD.bazel +++ b/src/test/java/com/google/api/generator/engine/ast/BUILD.bazel @@ -9,6 +9,7 @@ TESTS = [ "EnumRefExprTest", "ExprStatementTest", "ForStatementTest", + "GeneralForStatementTest", "IdentifierNodeTest", "IfStatementTest", "InstanceofExprTest", diff --git a/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java new file mode 100644 index 0000000000..4d544d91aa --- /dev/null +++ b/src/test/java/com/google/api/generator/engine/ast/GeneralForStatementTest.java @@ -0,0 +1,88 @@ +// 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.engine.ast; + +import static org.junit.Assert.assertThrows; + +import java.util.Arrays; +import java.util.Collections; +import org.junit.Test; + +public class GeneralForStatementTest { + + @Test + public void validGeneralForStatement_basicIsDecl() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); + VariableExpr variableExpr = + VariableExpr.builder().setVariable(variable).setIsDecl(true).build(); + + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").build(); + + GeneralForStatement.incrementWith( + variableExpr, maxSizeExpr, Arrays.asList(createBodyStatement())); + } + + @Test + public void validGeneralForStatement_basicIsNotDecl() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); + VariableExpr variableExpr = + VariableExpr.builder().setVariable(variable).setIsDecl(false).build(); + + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").build(); + + GeneralForStatement.incrementWith( + variableExpr, maxSizeExpr, Arrays.asList(createBodyStatement())); + } + + @Test + public void validGeneralForStatement_emptyBody() { + Variable variable = Variable.builder().setName("i").setType(TypeNode.INT).build(); + VariableExpr variableExpr = + VariableExpr.builder().setVariable(variable).setIsDecl(false).build(); + + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").build(); + + GeneralForStatement.incrementWith(variableExpr, maxSizeExpr, Collections.emptyList()); + } + + @Test + public void invalidForStatement() { + Variable variable = Variable.builder().setName("str").setType(TypeNode.STRING).build(); + VariableExpr variableExpr = + VariableExpr.builder().setVariable(variable).setScope(ScopeNode.PRIVATE).build(); + MethodInvocationExpr maxSizeExpr = + MethodInvocationExpr.builder().setMethodName("maxSize").build(); + + assertThrows( + IllegalStateException.class, + () -> + GeneralForStatement.incrementWith(variableExpr, maxSizeExpr, Collections.emptyList())); + } + + private static Statement createBodyStatement() { + Variable variable = Variable.builder().setName("x").setType(TypeNode.INT).build(); + VariableExpr variableExpr = + VariableExpr.builder().setVariable(variable).setIsDecl(true).build(); + + Variable anotherVariable = Variable.builder().setName("y").setType(TypeNode.INT).build(); + Expr valueExpr = VariableExpr.builder().setVariable(anotherVariable).build(); + + return ExprStatement.withExpr( + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(valueExpr).build()); + } +} diff --git a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java index a3e2705bea..ec6ac557f5 100644 --- a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java @@ -30,6 +30,7 @@ import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; import com.google.api.generator.engine.ast.ForStatement; +import com.google.api.generator.engine.ast.GeneralForStatement; import com.google.api.generator.engine.ast.IdentifierNode; import com.google.api.generator.engine.ast.IfStatement; import com.google.api.generator.engine.ast.InstanceofExpr; @@ -1298,6 +1299,26 @@ public void writeForStatement() { "for (String str : getSomeStrings()) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); } + @Test + public void writeGeneralForStatement_basic() { + AssignmentExpr assignExpr = createAssignmentExpr("x", "3", TypeNode.INT); + Statement assignExprStatement = ExprStatement.withExpr(assignExpr); + List body = Arrays.asList(assignExprStatement, assignExprStatement); + + VariableExpr localVarExpr = createVariableDeclExpr("i", TypeNode.INT); + Expr maxSizeExpr = MethodInvocationExpr.builder().setMethodName("maxSize").build(); + + GeneralForStatement forStatement = + GeneralForStatement.incrementWith(localVarExpr, maxSizeExpr, body); + + forStatement.accept(writerVisitor); + assertEquals( + writerVisitor.write(), + String.format( + "%s%s%s%s", + "for (int i = 0; i < maxSize(); i++) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); + } + @Test public void writeTryCatchStatement_simple() { Reference exceptionReference = ConcreteReference.withClazz(IllegalArgumentException.class); From 23b443e2521228ba237f2b583d76e88ba81586c9 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 1 Sep 2020 18:00:11 -0700 Subject: [PATCH 08/15] feat: add splitRseponse to batching desc. in ServiceStubSettings --- .../composer/BatchingDescriptorComposer.java | 202 +++++++++++++++++- .../BatchingDescriptorComposerTest.java | 24 +++ .../ServiceStubSettingsClassComposerTest.java | 33 +++ 3 files changed, 258 insertions(+), 1 deletion(-) 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 index bcdb809df3..f5b601a947 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java @@ -24,15 +24,20 @@ import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; import com.google.api.generator.engine.ast.ForStatement; +import com.google.api.generator.engine.ast.GeneralForStatement; 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.PrimitiveValue; import com.google.api.generator.engine.ast.Reference; import com.google.api.generator.engine.ast.ScopeNode; +import com.google.api.generator.engine.ast.Statement; import com.google.api.generator.engine.ast.TypeNode; +import com.google.api.generator.engine.ast.ValueExpr; import com.google.api.generator.engine.ast.Variable; import com.google.api.generator.engine.ast.VariableExpr; +import com.google.api.generator.gapic.model.Field; import com.google.api.generator.gapic.model.GapicBatchingSettings; import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.Method; @@ -59,6 +64,7 @@ public class BatchingDescriptorComposer { private static final TypeNode PARTITION_KEY_TYPE = toType(PartitionKey.class); private static final String ADD_ALL_METHOD_PATTERN = "addAll%s"; + private static final String BATCH_FOO_INDEX_PATTERN = "batch%sIndex"; private static final String GET_LIST_METHOD_PATTERN = "get%sList"; private static final String GET_COUNT_METHOD_PATTERN = "get%sCount"; @@ -67,7 +73,7 @@ public static Expr createBatchingDescriptorFieldDeclExpr( List javaMethods = new ArrayList<>(); javaMethods.add(createGetBatchPartitionKeyMethod(method, batchingSettings, messageTypes)); javaMethods.add(createGetRequestBuilderMethod(method, batchingSettings)); - + javaMethods.add(createSplitResponseMethod(method, batchingSettings, messageTypes)); javaMethods.add(createSplitExceptionMethod(method)); javaMethods.add(createCountElementsMethod(method, batchingSettings)); javaMethods.add(createCountByteSMethod(method)); @@ -236,6 +242,200 @@ private static MethodDefinition createGetRequestBuilderMethod( .build(); } + private static MethodDefinition createSplitResponseMethod( + Method method, GapicBatchingSettings batchingSettings, Map messageTypes) { + VariableExpr batchResponseVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(method.outputType()).setName("batchResponse").build()); + + TypeNode batchedRequestIssuerType = toType(BATCHED_REQUEST_ISSUER_REF, method.outputType()); + TypeNode batchVarType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(Collection.class) + .setGenerics( + Arrays.asList( + ConcreteReference.wildcardWithUpperBound( + batchedRequestIssuerType.reference()))) + .build()); + VariableExpr batchVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(batchVarType).setName("batch").build()); + + VariableExpr responderVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(batchedRequestIssuerType).setName("responder").build()); + + String upperCamelBatchedFieldName = + JavaStyle.toUpperCamelCase(batchingSettings.batchedFieldName()); + VariableExpr batchMessageIndexVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(TypeNode.INT).setName("batchMessageIndex").build()); + + VariableExpr subresponseElementsVarExpr = null; + boolean hasSubresponseField = batchingSettings.subresponseFieldName() != null; + + List outerForBody = new ArrayList<>(); + if (hasSubresponseField) { + Message outputMessage = messageTypes.get(method.outputType().reference().name()); + Preconditions.checkNotNull( + outputMessage, String.format("Output message not found for RPC %s", method.name())); + + Field subresponseElementField = + outputMessage.fieldMap().get(batchingSettings.subresponseFieldName()); + Preconditions.checkNotNull( + subresponseElementField, + String.format( + "Subresponse field %s not found in message %s", + batchingSettings.subresponseFieldName(), outputMessage.name())); + TypeNode subresponseElementType = subresponseElementField.type(); + subresponseElementsVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setType(subresponseElementType) + .setName("subresponseElements") + .build()); + + VariableExpr subresponseCountVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(TypeNode.LONG).setName("subresponseCount").build()); + + outerForBody.add( + ExprStatement.withExpr( + AssignmentExpr.builder() + .setVariableExpr(subresponseElementsVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr( + NewObjectExpr.builder() + .setType( + TypeNode.withReference(ConcreteReference.withClazz(ArrayList.class))) + .setIsGeneric(true) + .build()) + .build())); + + String getFooCountMethodName = "getMessageCount"; + outerForBody.add( + ExprStatement.withExpr( + AssignmentExpr.builder() + .setVariableExpr(subresponseCountVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(responderVarExpr) + .setMethodName(getFooCountMethodName) + .setReturnType(subresponseCountVarExpr.type()) + .build()) + .build())); + + List innerSubresponseForExprs = new ArrayList<>(); + String getSubresponseFieldMethodName = + String.format( + "get%s", JavaStyle.toUpperCamelCase(batchingSettings.subresponseFieldName())); + Expr addMethodArgExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(batchResponseVarExpr) + .setMethodName(getSubresponseFieldMethodName) + .setArguments(batchMessageIndexVarExpr) + .build(); + innerSubresponseForExprs.add( + MethodInvocationExpr.builder() + .setExprReferenceExpr(subresponseElementsVarExpr) + .setMethodName("add") + .setArguments(addMethodArgExpr) + .build()); + // TODO(miraleung): Increment batchMessageIndexVarExpr. + + VariableExpr forIndexVarExpr = + VariableExpr.withVariable(Variable.builder().setType(TypeNode.INT).setName("i").build()); + GeneralForStatement innerSubresponseForStatement = + GeneralForStatement.incrementWith( + forIndexVarExpr, + subresponseCountVarExpr, + innerSubresponseForExprs.stream() + .map(e -> ExprStatement.withExpr(e)) + .collect(Collectors.toList())); + + outerForBody.add(innerSubresponseForStatement); + } + + TypeNode responseType = method.outputType(); + Expr responseBuilderExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(responseType) + .setMethodName("newBuilder") + .build(); + if (hasSubresponseField) { + Preconditions.checkNotNull( + subresponseElementsVarExpr, + String.format( + "subresponseElements variable should not be null for method %s", method.name())); + + responseBuilderExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(responseBuilderExpr) + .setMethodName( + String.format( + "addAll%s", + JavaStyle.toUpperCamelCase(batchingSettings.subresponseFieldName()))) + .setArguments(subresponseElementsVarExpr) + .build(); + } + responseBuilderExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(responseBuilderExpr) + .setMethodName("build") + .setReturnType(responseType) + .build(); + + VariableExpr responseVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(responseType).setName("response").build()); + outerForBody.add( + ExprStatement.withExpr( + AssignmentExpr.builder() + .setVariableExpr(responseVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr(responseBuilderExpr) + .build())); + + outerForBody.add( + ExprStatement.withExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(responderVarExpr) + .setMethodName("setResponse") + .setArguments(responseVarExpr) + .build())); + + ForStatement outerForStatement = + ForStatement.builder() + .setLocalVariableExpr(responderVarExpr.toBuilder().setIsDecl(true).build()) + .setCollectionExpr(batchVarExpr) + .setBody(outerForBody) + .build(); + + List bodyStatements = new ArrayList<>(); + if (hasSubresponseField) { + bodyStatements.add( + ExprStatement.withExpr( + AssignmentExpr.builder() + .setVariableExpr(batchMessageIndexVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr( + ValueExpr.withValue( + PrimitiveValue.builder().setType(TypeNode.INT).setValue("0").build())) + .build())); + } + bodyStatements.add(outerForStatement); + + return MethodDefinition.builder() + .setIsOverride(true) + .setScope(ScopeNode.PUBLIC) + .setReturnType(TypeNode.VOID) + .setName("splitResponse") + .setArguments( + Arrays.asList(batchResponseVarExpr, batchVarExpr).stream() + .map(v -> v.toBuilder().setIsDecl(true).build()) + .collect(Collectors.toList())) + .setBody(bodyStatements) + .build(); + } + private static MethodDefinition createSplitExceptionMethod(Method method) { VariableExpr throwableVarExpr = VariableExpr.withVariable( 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 index e412e0a832..844f8b3cbd 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java @@ -129,6 +129,22 @@ public void batchingDescriptor_hasSubresponseField() { "};\n", "}\n", "@Override\n", + "public void splitResponse(", + "PublishResponse batchResponse, ", + "Collection> batch) {\n", + "int batchMessageIndex = 0;\n", + "for (BatchedRequestIssuer responder : batch) {\n", + "List subresponseElements = new ArrayList<>();\n", + "long subresponseCount = responder.getMessageCount();\n", + "for (int i = 0; i < subresponseCount; i++) {\n", + "subresponseElements.add(batchResponse.getMessageIds(batchMessageIndex));\n", + "}\n", + "PublishResponse response = ", + "PublishResponse.newBuilder().addAllMessageIds(subresponseElements).build();\n", + "responder.setResponse(response);\n", + "}\n", + "}\n", + "@Override\n", "public void splitException(", "Throwable throwable, ", "Collection> batch) {\n", @@ -226,6 +242,14 @@ public void batchingDescriptor_noSubresponseField() { "};\n", "}\n", "@Override\n", + "public void splitResponse(WriteLogEntriesResponse batchResponse, ", + "Collection> batch) {\n", + "for (BatchedRequestIssuer responder : batch) {\n", + "WriteLogEntriesResponse response = WriteLogEntriesResponse.newBuilder().build();\n", + "responder.setResponse(response);\n", + "}\n", + "}\n", + "@Override\n", "public void splitException(", "Throwable throwable, ", "Collection> batch) {\n", 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 10456faed3..52b025ead1 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 @@ -999,6 +999,19 @@ private static List parseServices( + " }\n" + "\n" + " @Override\n" + + " public void splitResponse(\n" + + " WriteLogEntriesResponse batchResponse,\n" + + " Collection>" + + " batch) {\n" + + " for (BatchedRequestIssuer responder : batch)" + + " {\n" + + " WriteLogEntriesResponse response =" + + " WriteLogEntriesResponse.newBuilder().build();\n" + + " responder.setResponse(response);\n" + + " }\n" + + " }\n" + + "\n" + + " @Override\n" + " public void splitException(\n" + " Throwable throwable,\n" + " Collection>" @@ -1397,6 +1410,7 @@ private static List parseServices( + "import com.google.pubsub.v1.Topic;\n" + "import com.google.pubsub.v1.UpdateTopicRequest;\n" + "import java.io.IOException;\n" + + "import java.util.ArrayList;\n" + "import java.util.Collection;\n" + "import java.util.List;\n" + "import java.util.Objects;\n" @@ -1662,6 +1676,25 @@ private static List parseServices( + " }\n" + "\n" + " @Override\n" + + " public void splitResponse(\n" + + " PublishResponse batchResponse,\n" + + " Collection> batch) {\n" + + " int batchMessageIndex = 0;\n" + + " for (BatchedRequestIssuer responder : batch) {\n" + + " List subresponseElements = new ArrayList<>();\n" + + " long subresponseCount = responder.getMessageCount();\n" + + " for (int i = 0; i < subresponseCount; i++) {\n" + + " " + + " subresponseElements.add(batchResponse.getMessageIds(batchMessageIndex));\n" + + " }\n" + + " PublishResponse response =\n" + + " " + + " PublishResponse.newBuilder().addAllMessageIds(subresponseElements).build();\n" + + " responder.setResponse(response);\n" + + " }\n" + + " }\n" + + "\n" + + " @Override\n" + " public void splitException(\n" + " Throwable throwable,\n" + " Collection> batch) {\n" From 381c6e4aba1a76995b547b38127c6e5603a1d458 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 2 Sep 2020 14:44:16 -0700 Subject: [PATCH 09/15] fix: output srcjar path --- src/main/java/com/google/api/generator/Main.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/generator/Main.java b/src/main/java/com/google/api/generator/Main.java index 42a02a40cf..a1db4e4ab4 100644 --- a/src/main/java/com/google/api/generator/Main.java +++ b/src/main/java/com/google/api/generator/Main.java @@ -32,7 +32,7 @@ public static void main(String[] args) ExtensionRegistry registry = ExtensionRegistry.newInstance(); registerAllExtensions(registry); CodeGeneratorRequest request = CodeGeneratorRequest.parseFrom(System.in, registry); - String outputFilePath = String.format("%stemp-gen.srcjar", request.getParameter()); + String outputFilePath = "temp-gen.srcjar"; CodeGeneratorResponse response = Generator.generateGapic(request, outputFilePath); response.writeTo(System.out); } From f70d7560f2512610e68fed948ef51f7c3c9d5b73 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 3 Sep 2020 13:22:16 -0700 Subject: [PATCH 10/15] feat: add method comments to ServiceStubSettings, fix AST formatting --- .../generator/engine/ast/ClassDefinition.java | 13 +- .../generator/engine/ast/JavaDocComment.java | 5 + .../engine/ast/MethodDefinition.java | 4 + .../engine/writer/JavaWriterVisitor.java | 3 + .../CommentComposer.java} | 26 ++-- .../generator/gapic/composer/Composer.java | 3 +- .../ServiceStubSettingsClassComposer.java | 99 ++++++++++++---- .../ServiceStubSettingsCommentComposer.java | 82 +++++++++++++ .../engine/writer/JavaWriterVisitorTest.java | 24 ++-- .../BatchingDescriptorComposerTest.java | 40 +++++-- ...rviceCallableFactoryClassComposerTest.java | 1 + .../ServiceSettingsClassComposerTest.java | 2 + .../ServiceStubClassComposerTest.java | 1 + .../ServiceStubSettingsClassComposerTest.java | 112 ++++++++++++++++++ 14 files changed, 355 insertions(+), 60 deletions(-) rename src/main/java/com/google/api/generator/gapic/{utils/ApacheLicense.java => composer/CommentComposer.java} (60%) create mode 100644 src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsCommentComposer.java diff --git a/src/main/java/com/google/api/generator/engine/ast/ClassDefinition.java b/src/main/java/com/google/api/generator/engine/ast/ClassDefinition.java index 2a5fd9b685..4b2a666a4b 100644 --- a/src/main/java/com/google/api/generator/engine/ast/ClassDefinition.java +++ b/src/main/java/com/google/api/generator/engine/ast/ClassDefinition.java @@ -17,6 +17,7 @@ import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import javax.annotation.Nullable; @@ -85,8 +86,16 @@ public static Builder builder() { @AutoValue.Builder public abstract static class Builder { + public Builder setFileHeader(CommentStatement... headerComments) { + return setFileHeader(Arrays.asList(headerComments)); + } + public abstract Builder setFileHeader(List fileHeader); + public Builder setHeaderCommentStatements(CommentStatement... comments) { + return setHeaderCommentStatements(Arrays.asList(comments)); + } + public abstract Builder setHeaderCommentStatements( List headerCommentStatements); @@ -165,7 +174,9 @@ public ClassDefinition build() { for (Statement statement : classDef.statements()) { // TODO(xiaozhenliu): Add CommentStatement check here. Preconditions.checkState( - statement instanceof ExprStatement || statement instanceof BlockStatement, + statement instanceof CommentStatement + || statement instanceof ExprStatement + || statement instanceof BlockStatement, "Class statement type must be either an expression, block, or comment statement"); if (statement instanceof ExprStatement) { Expr expr = ((ExprStatement) statement).expression(); diff --git a/src/main/java/com/google/api/generator/engine/ast/JavaDocComment.java b/src/main/java/com/google/api/generator/engine/ast/JavaDocComment.java index 0bd2b67cd4..5e0a4665e8 100644 --- a/src/main/java/com/google/api/generator/engine/ast/JavaDocComment.java +++ b/src/main/java/com/google/api/generator/engine/ast/JavaDocComment.java @@ -28,6 +28,11 @@ public abstract class JavaDocComment implements Comment { @Override public abstract String comment(); + // Convenience helper for simple comments. + public static JavaDocComment withComment(String comment) { + return builder().addComment(comment).build(); + } + public static Builder builder() { return new AutoValue_JavaDocComment.Builder(); } diff --git a/src/main/java/com/google/api/generator/engine/ast/MethodDefinition.java b/src/main/java/com/google/api/generator/engine/ast/MethodDefinition.java index 285fc898dd..21feb7a1db 100644 --- a/src/main/java/com/google/api/generator/engine/ast/MethodDefinition.java +++ b/src/main/java/com/google/api/generator/engine/ast/MethodDefinition.java @@ -120,6 +120,10 @@ public abstract static class Builder { public abstract Builder setName(String name); + public Builder setHeaderCommentStatements(CommentStatement... comments) { + return setHeaderCommentStatements(Arrays.asList(comments)); + } + public abstract Builder setHeaderCommentStatements( List headeCommentStatements); diff --git a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java index f9ee6141e6..a51e08c8a4 100644 --- a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java @@ -707,6 +707,7 @@ public void visit(MethodDefinition methodDefinition) { rightBrace(); newline(); + newline(); } @Override @@ -778,7 +779,9 @@ public void visit(ClassDefinition classDefinition) { newline(); statements(classDefinition.statements()); + newline(); methods(classDefinition.methods()); + newline(); classes(classDefinition.nestedClasses()); rightBrace(); diff --git a/src/main/java/com/google/api/generator/gapic/utils/ApacheLicense.java b/src/main/java/com/google/api/generator/gapic/composer/CommentComposer.java similarity index 60% rename from src/main/java/com/google/api/generator/gapic/utils/ApacheLicense.java rename to src/main/java/com/google/api/generator/gapic/composer/CommentComposer.java index 00de28f7e5..8457288b82 100644 --- a/src/main/java/com/google/api/generator/gapic/utils/ApacheLicense.java +++ b/src/main/java/com/google/api/generator/gapic/composer/CommentComposer.java @@ -12,15 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.api.generator.gapic.utils; +package com.google.api.generator.gapic.composer; import com.google.api.generator.engine.ast.BlockComment; import com.google.api.generator.engine.ast.CommentStatement; -import java.util.Arrays; -import java.util.List; +import com.google.api.generator.engine.ast.LineComment; -public class ApacheLicense { - private static final String fileHeaderString = +public class CommentComposer { + private static final String APACHE_LICENSE_STRING = "Copyright 2020 Google LLC\n\n" + "Licensed under the Apache License, Version 2.0 (the \"License\");\n" + "you may not use this file except in compliance with the License.\n" @@ -32,6 +31,19 @@ public class ApacheLicense { + "See the License for the specific language governing permissions and\n" + "limitations under the License."; - public static final List APACHE_LICENSE_COMMENT_STATEMENT = - Arrays.asList(CommentStatement.withComment(BlockComment.withComment(fileHeaderString))); + private static final String AUTO_GENERATED_CLASS_DISCLAIMER_STRING = + "AUTO-GENERATED DOCUMENTATION AND CLASS."; + + private static final String AUTO_GENERATED_METHOD_DISCLAIMER_STRING = + "AUTO-GENERATED DOCUMENTATION AND METHOD."; + + public static final CommentStatement APACHE_LICENSE_COMMENT = + CommentStatement.withComment(BlockComment.withComment(APACHE_LICENSE_STRING)); + + public static final CommentStatement AUTO_GENERATED_CLASS_COMMENT = + CommentStatement.withComment(LineComment.withComment(AUTO_GENERATED_CLASS_DISCLAIMER_STRING)); + + public static final CommentStatement AUTO_GENERATED_METHOD_COMMENT = + CommentStatement.withComment( + LineComment.withComment(AUTO_GENERATED_METHOD_DISCLAIMER_STRING)); } diff --git a/src/main/java/com/google/api/generator/gapic/composer/Composer.java b/src/main/java/com/google/api/generator/gapic/composer/Composer.java index c193519023..ca50e1892d 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/Composer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/Composer.java @@ -23,7 +23,6 @@ import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.model.Service; -import com.google.api.generator.gapic.utils.ApacheLicense; import com.google.common.annotations.VisibleForTesting; import java.util.ArrayList; import java.util.List; @@ -117,7 +116,7 @@ protected static List addApacheLicense(List gapicClassLi gapicClass .classDefinition() .toBuilder() - .setFileHeader(ApacheLicense.APACHE_LICENSE_COMMENT_STATEMENT) + .setFileHeader(CommentComposer.APACHE_LICENSE_COMMENT) .build(); return GapicClass.create(gapicClass.kind(), classWithHeader); }) 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 4b8960b16f..e7b4b87072 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 @@ -103,6 +103,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.function.BiFunction; import java.util.function.Function; import java.util.stream.Collectors; import javax.annotation.Generated; @@ -123,6 +124,8 @@ public class ServiceStubSettingsClassComposer { "RETRYABLE_CODE_DEFINITIONS"; private static final String NESTED_RETRY_PARAM_DEFINITIONS_VAR_NAME = "RETRY_PARAM_DEFINITIONS"; private static final String STUB_PATTERN = "%sStub"; + + private static final String OPERATION_SETTINGS_LITERAL = "OperationSettings"; private static final String SETTINGS_LITERAL = "Settings"; private static final String LEFT_BRACE = "{"; @@ -160,6 +163,7 @@ public GapicClass generate( ClassDefinition classDef = ClassDefinition.builder() .setPackageString(pakkage) + .setHeaderCommentStatements(CommentComposer.AUTO_GENERATED_CLASS_COMMENT) .setAnnotations(createClassAnnotations()) .setScope(ScopeNode.PUBLIC) .setName(className) @@ -230,9 +234,11 @@ private static List createClassStatements( Map methodSettingsMemberVarExprs, Map messageTypes, Map types) { - List memberVarExprs = new ArrayList<>(); + Function exprToStatementFn = e -> ExprStatement.withExpr(e); + List statements = new ArrayList<>(); // Assign DEFAULT_SERVICE_SCOPES. + statements.add(ServiceStubSettingsCommentComposer.DEFAULT_SCOPES_COMMENT); VariableExpr defaultServiceScopesDeclVarExpr = DEFAULT_SERVICE_SCOPES_VAR_EXPR .toBuilder() @@ -263,38 +269,43 @@ private static List createClassStatements( .setReturnType(DEFAULT_SERVICE_SCOPES_VAR_EXPR.type()) .build(); - memberVarExprs.add( - AssignmentExpr.builder() - .setVariableExpr(defaultServiceScopesDeclVarExpr) - .setValueExpr(listBuilderExpr) - .build()); + statements.add( + exprToStatementFn.apply( + AssignmentExpr.builder() + .setVariableExpr(defaultServiceScopesDeclVarExpr) + .setValueExpr(listBuilderExpr) + .build())); // Declare settings members. - memberVarExprs.addAll( + statements.addAll( methodSettingsMemberVarExprs.values().stream() .map( v -> - v.toBuilder() - .setIsDecl(true) - .setScope(ScopeNode.PRIVATE) - .setIsFinal(true) - .build()) + exprToStatementFn.apply( + v.toBuilder() + .setIsDecl(true) + .setScope(ScopeNode.PRIVATE) + .setIsFinal(true) + .build())) .collect(Collectors.toList())); - memberVarExprs.addAll( - createPagingStaticAssignExprs(service, serviceConfig, messageTypes, types)); + statements.addAll( + createPagingStaticAssignExprs(service, serviceConfig, messageTypes, types).stream() + .map(e -> exprToStatementFn.apply(e)) + .collect(Collectors.toList())); for (Method method : service.methods()) { Optional batchingSettingOpt = serviceConfig.getBatchingSetting(service, method); if (batchingSettingOpt.isPresent()) { - memberVarExprs.add( - BatchingDescriptorComposer.createBatchingDescriptorFieldDeclExpr( - method, batchingSettingOpt.get(), messageTypes)); + statements.add( + exprToStatementFn.apply( + BatchingDescriptorComposer.createBatchingDescriptorFieldDeclExpr( + method, batchingSettingOpt.get(), messageTypes))); } } - return memberVarExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList()); + return statements; } private static List createPagingStaticAssignExprs( @@ -703,7 +714,6 @@ private static List createClassMethods( javaMethods.addAll(createDefaultHelperAndGetterMethods(service, types)); javaMethods.addAll(createBuilderHelperMethods(service, types)); javaMethods.add(createClassConstructor(service, methodSettingsMemberVarExprs, types)); - // TODO(miraleung): Fill this out. return javaMethods; } @@ -712,6 +722,9 @@ private static List createMethodSettingsGetterMethods( Function, MethodDefinition> varToMethodFn = e -> MethodDefinition.builder() + .setHeaderCommentStatements( + ServiceStubSettingsCommentComposer.createCallSettingsGetterComment( + getMethodNameFromSettingsVarName(e.getKey()))) .setScope(ScopeNode.PUBLIC) .setReturnType(e.getValue().type()) .setName(e.getKey()) @@ -806,6 +819,8 @@ private static List createDefaultHelperAndGetterMethods( ConcreteReference.withClazz(InstantiatingExecutorProvider.Builder.class)); javaMethods.add( MethodDefinition.builder() + .setHeaderCommentStatements( + ServiceStubSettingsCommentComposer.DEFAULT_EXECUTOR_PROVIDER_BUILDER_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(returnType) @@ -822,6 +837,8 @@ private static List createDefaultHelperAndGetterMethods( returnType = TypeNode.STRING; javaMethods.add( MethodDefinition.builder() + .setHeaderCommentStatements( + ServiceStubSettingsCommentComposer.DEFAULT_SERVICE_ENDPOINT_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(returnType) @@ -838,6 +855,8 @@ private static List createDefaultHelperAndGetterMethods( .build()); javaMethods.add( MethodDefinition.builder() + .setHeaderCommentStatements( + ServiceStubSettingsCommentComposer.DEFAULT_SERVICE_SCOPES_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(returnType) @@ -863,6 +882,9 @@ private static List createDefaultHelperAndGetterMethods( .build(); javaMethods.add( MethodDefinition.builder() + .setHeaderCommentStatements( + ServiceStubSettingsCommentComposer + .DEFAULT_CREDENTIALS_PROVIDER_BUILDER_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(returnType) @@ -893,6 +915,9 @@ private static List createDefaultHelperAndGetterMethods( .build(); javaMethods.add( MethodDefinition.builder() + .setHeaderCommentStatements( + ServiceStubSettingsCommentComposer + .DEFAULT_GRPC_TRANSPORT_PROVIDER_BUILDER_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(returnType) @@ -993,6 +1018,8 @@ private static List createBuilderHelperMethods( final TypeNode builderReturnType = types.get(NESTED_BUILDER_CLASS_NAME); javaMethods.add( MethodDefinition.builder() + .setHeaderCommentStatements( + ServiceStubSettingsCommentComposer.NEW_BUILDER_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(builderReturnType) @@ -1016,6 +1043,8 @@ private static List createBuilderHelperMethods( .build()); javaMethods.add( MethodDefinition.builder() + .setHeaderCommentStatements( + ServiceStubSettingsCommentComposer.NEW_BUILDER_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(builderReturnType) @@ -1027,6 +1056,8 @@ private static List createBuilderHelperMethods( // Create the toBuilder method. javaMethods.add( MethodDefinition.builder() + .setHeaderCommentStatements( + ServiceStubSettingsCommentComposer.TO_BUILDER_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setReturnType(builderReturnType) .setName("toBuilder") @@ -1116,6 +1147,9 @@ private static ClassDefinition createNestedBuilderClass( // TODO(miraleung): Fill this out. return ClassDefinition.builder() .setIsNested(true) + .setHeaderCommentStatements( + ServiceStubSettingsCommentComposer.createBuilderClassComment( + getThisClassName(service.name()))) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setName(className) @@ -1333,12 +1367,10 @@ private static List createNestedClassConstructorMethods( // Name is fooBarSettings. VariableExpr varExpr = e.getValue(); TypeNode varType = varExpr.type(); - String methodName = e.getKey(); Preconditions.checkState( - methodName.endsWith(SETTINGS_LITERAL), - String.format("%s expected to end with \"Settings\"", methodName)); - methodName = - methodName.substring(0, methodName.length() - SETTINGS_LITERAL.length()); + e.getKey().endsWith(SETTINGS_LITERAL), + String.format("%s expected to end with \"Settings\"", e.getKey())); + String methodName = getMethodNameFromSettingsVarName(e.getKey()); if (!isPagedCallSettingsBuilderFn.apply(varType)) { if (!isBatchingCallSettingsBuilderFn.apply(varType)) { @@ -1621,8 +1653,9 @@ private static MethodDefinition createNestedClassApplyToAllUnaryMethodsMethod( TypeNode returnType = types.get(NESTED_BUILDER_CLASS_NAME); Expr returnExpr = ValueExpr.withValue(ThisObjectValue.withType(returnType)); - // TODO(miraleung): Add major ver note. return MethodDefinition.builder() + .setHeaderCommentStatements( + ServiceStubSettingsCommentComposer.APPLY_TO_ALL_UNARY_METHODS_METHOD_COMMENTS) .setScope(ScopeNode.PUBLIC) .setReturnType(returnType) .setName(methodName) @@ -1669,6 +1702,9 @@ private static List createNestedClassSettingsBuilderGetterMeth isOperationCallSettingsBuilderFn.apply(settingsVarExpr.type()); javaMethods.add( MethodDefinition.builder() + .setHeaderCommentStatements( + ServiceStubSettingsCommentComposer.createCallSettingsBuilderGetterComment( + getMethodNameFromSettingsVarName(varName))) .setAnnotations(isOperationCallSettings ? lroBetaAnnotations : ImmutableList.of()) .setScope(ScopeNode.PUBLIC) .setReturnType(settingsVarExpr.type()) @@ -1964,4 +2000,17 @@ private static TypeNode getOperationCallSettingsType(Method method, boolean isSe .setGenerics(generics) .build()); } + + /** Turns a name like "waitSettings" or "waitOperationSettings" into "wait". */ + private static String getMethodNameFromSettingsVarName(String settingsVarName) { + BiFunction methodNameSubstrFn = + (s, literal) -> s.substring(0, s.length() - literal.length()); + if (settingsVarName.endsWith(OPERATION_SETTINGS_LITERAL)) { + return methodNameSubstrFn.apply(settingsVarName, OPERATION_SETTINGS_LITERAL); + } + if (settingsVarName.endsWith(SETTINGS_LITERAL)) { + return methodNameSubstrFn.apply(settingsVarName, SETTINGS_LITERAL); + } + return settingsVarName; + } } diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsCommentComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsCommentComposer.java new file mode 100644 index 0000000000..659c37d175 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsCommentComposer.java @@ -0,0 +1,82 @@ +// 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.generator.engine.ast.CommentStatement; +import com.google.api.generator.engine.ast.JavaDocComment; +import com.google.api.generator.engine.ast.LineComment; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +class ServiceStubSettingsCommentComposer { + private static final String BUILDER_CLASS_DOC_PATTERN = "Builder for %s."; + private static final String CALL_SETTINGS_METHOD_DOC_PATTERN = + "Returns the object with the settings used for calls to %s."; + private static final String CALL_SETTINGS_BUILDER_METHOD_DOC_PATTERN = + "Returns the builder for the settings used for calls to %s."; + + static final CommentStatement DEFAULT_SCOPES_COMMENT = + toSimpleComment("The default scopes of the service."); + + static final CommentStatement DEFAULT_EXECUTOR_PROVIDER_BUILDER_METHOD_COMMENT = + toSimpleComment("Returns a builder for the default ExecutorProvider for this service."); + static final CommentStatement DEFAULT_SERVICE_ENDPOINT_METHOD_COMMENT = + toSimpleComment("Returns the default service endpoint."); + static final CommentStatement DEFAULT_SERVICE_SCOPES_METHOD_COMMENT = + toSimpleComment("Returns the default service scopes."); + + static final CommentStatement DEFAULT_CREDENTIALS_PROVIDER_BUILDER_METHOD_COMMENT = + toSimpleComment("Returns a builder for the default credentials for this service."); + + static final CommentStatement DEFAULT_GRPC_TRANSPORT_PROVIDER_BUILDER_METHOD_COMMENT = + toSimpleComment("Returns a builder for the default ChannelProvider for this service."); + + static final CommentStatement NEW_BUILDER_METHOD_COMMENT = + toSimpleComment("Returns a new builder for this class."); + + static final CommentStatement TO_BUILDER_METHOD_COMMENT = + toSimpleComment("Returns a builder containing all the values of this settings class."); + + static final List APPLY_TO_ALL_UNARY_METHODS_METHOD_COMMENTS = + Arrays.asList( + LineComment.withComment("NEXT_MAJOR_VER: remove 'throws Exception'."), + JavaDocComment.builder() + .addComment( + "Applies the given settings updater function to all of the unary API methods" + + " in this service.") + .addParagraph( + "Note: This method does not support applying settings to streaming methods.") + .build()) + .stream() + .map(c -> CommentStatement.withComment(c)) + .collect(Collectors.toList()); + + static CommentStatement createCallSettingsGetterComment(String javaMethodName) { + return toSimpleComment(String.format(CALL_SETTINGS_METHOD_DOC_PATTERN, javaMethodName)); + } + + static CommentStatement createBuilderClassComment(String outerClassName) { + return toSimpleComment(String.format(BUILDER_CLASS_DOC_PATTERN, outerClassName)); + } + + static CommentStatement createCallSettingsBuilderGetterComment(String javaMethodName) { + return toSimpleComment(String.format(CALL_SETTINGS_BUILDER_METHOD_DOC_PATTERN, javaMethodName)); + } + + private static CommentStatement toSimpleComment(String comment) { + return CommentStatement.withComment(JavaDocComment.withComment(comment)); + } +} diff --git a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java index ec6ac557f5..b5a68783b0 100644 --- a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java @@ -854,7 +854,7 @@ public void writeAnonymousClassExpr_basic() { "new Runnable() {\n", "@Override\n", "public void run() {\n", - "boolean foobar = false;\n}\n}")); + "boolean foobar = false;\n}\n\n}")); } @Test @@ -899,7 +899,7 @@ public void writeAnonymousClassExpr_withStatementsMethods() { "private static final String s = \"foo\";\n", "@Override\n", "public void run() {\n", - "int x = 3;\n}\n}"); + "int x = 3;\n}\n\n}"); assertEquals(writerVisitor.write(), expected); } @@ -956,7 +956,7 @@ public void writeAnonymousClassExpr_generics() { "@Override\n", "public MethodDefinition apply(List arg) {\n", "return returnArg;\n", - "}\n}"); + "}\n\n}"); assertEquals(writerVisitor.write(), expected); } @@ -1470,7 +1470,7 @@ public void writeMethodDefinition_basic() { methodDefinition.accept(writerVisitor); assertEquals( writerVisitor.write(), - String.format("%s%s%s", "public void close() {\n", "int x = 3;\n", "}\n")); + String.format("%s%s%s", "public void close() {\n", "int x = 3;\n", "}\n\n")); } @Test @@ -1488,7 +1488,7 @@ public void writeMethodDefinition_constructor() { .build(); methodDefinition.accept(writerVisitor); - assertEquals(writerVisitor.write(), "public LibrarySettings() {\n}\n"); + assertEquals(writerVisitor.write(), "public LibrarySettings() {\n}\n\n"); } @Test @@ -1501,7 +1501,7 @@ public void writeMethodDefinition_basicEmptyBody() { .build(); methodDefinition.accept(writerVisitor); - assertEquals(writerVisitor.write(), "public void close() {\n}\n"); + assertEquals(writerVisitor.write(), "public void close() {\n}\n\n"); } @Test @@ -1519,7 +1519,7 @@ public void writeMethodDefinition_basicAbstract() { methodDefinition.accept(writerVisitor); assertEquals( writerVisitor.write(), - String.format("%s%s%s", "public abstract void close() {\n", "int x = 3;\n", "}\n")); + String.format("%s%s%s", "public abstract void close() {\n", "int x = 3;\n", "}\n\n")); } @Test @@ -1573,7 +1573,7 @@ public void writeMethodDefinition_withArgumentsAndReturnExpr() { "public int close(int x, int y) {\n", "boolean foobar = false;\n", "return 3;\n", - "}\n")); + "}\n\n")); } @Test @@ -1650,7 +1650,7 @@ public void writeMethodDefinition_withCommentsAnnotationsAndThrows() { "}\n", "boolean foobar = false;\n", "return 3;\n", - "}\n"); + "}\n\n"); assertEquals(writerVisitor.write(), expected); } @@ -1693,7 +1693,7 @@ public void writeMethodDefinition_templatedReturnTypeAndArguments() { createLines(3), "public Map close(Map x, Map y) {\n", "return foobar();\n", - "}\n")); + "}\n\n")); } @Test @@ -1884,7 +1884,7 @@ public void writeClassDefinition_commentsStatementsAndMethods() { " boolean foobar = false;\n", " }\n", "\n", - " private static class IAmANestedClass {\n", + " private static class IAmANestedClass {\n\n", " public boolean open() {\n", " return true;\n", " }\n", @@ -1942,7 +1942,7 @@ public void writeThisObjectValue_methodReturn() { methodDefinition.accept(writerVisitor); assertEquals( writerVisitor.write(), - String.format("public Student apply() {\n" + "return this;\n" + "}\n")); + String.format("public Student apply() {\n" + "return this;\n" + "}\n\n")); } @Test 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 index 844f8b3cbd..0780e85fe8 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java @@ -110,6 +110,7 @@ public void batchingDescriptor_hasSubresponseField() { "public PartitionKey getBatchPartitionKey(PublishRequest request) {\n", "return new PartitionKey(request.getTopic());\n", "}\n", + "\n", "@Override\n", "public RequestBuilder getRequestBuilder() {\n", "return new RequestBuilder() {\n", @@ -122,16 +123,18 @@ public void batchingDescriptor_hasSubresponseField() { "builder.addAllMessages(request.getMessagesList());\n", "}\n", "}\n", + "\n", "@Override\n", "public PublishRequest build() {\n", "return builder.build();\n", "}\n", + "\n", "};\n", "}\n", + "\n", "@Override\n", - "public void splitResponse(", - "PublishResponse batchResponse, ", - "Collection> batch) {\n", + "public void splitResponse(PublishResponse batchResponse, Collection> batch) {\n", "int batchMessageIndex = 0;\n", "for (BatchedRequestIssuer responder : batch) {\n", "List subresponseElements = new ArrayList<>();\n", @@ -139,28 +142,32 @@ public void batchingDescriptor_hasSubresponseField() { "for (int i = 0; i < subresponseCount; i++) {\n", "subresponseElements.add(batchResponse.getMessageIds(batchMessageIndex));\n", "}\n", - "PublishResponse response = ", - "PublishResponse.newBuilder().addAllMessageIds(subresponseElements).build();\n", + "PublishResponse response =" + + " PublishResponse.newBuilder().addAllMessageIds(subresponseElements).build();\n", "responder.setResponse(response);\n", "}\n", "}\n", + "\n", "@Override\n", - "public void splitException(", - "Throwable throwable, ", - "Collection> batch) {\n", + "public void splitException(Throwable throwable, Collection> batch) {\n", "for (BatchedRequestIssuer responder : batch) {\n", "responder.setException(throwable);\n", "}\n", "}\n", + "\n", "@Override\n", "public long countElements(PublishRequest request) {\n", "return request.getMessagesCount();\n", "}\n", + "\n", "@Override\n", "public long countBytes(PublishRequest request) {\n", "return request.getSerializedSize();\n", "}\n", + "\n", "}"); + assertEquals(expected, writerVisitor.write()); } @@ -223,6 +230,7 @@ public void batchingDescriptor_noSubresponseField() { "return new PartitionKey(request.getLogName(), request.getResource()," + " request.getLabels());\n", "}\n", + "\n", "@Override\n", "public RequestBuilder getRequestBuilder() {\n", "return new RequestBuilder() {\n", @@ -235,36 +243,42 @@ public void batchingDescriptor_noSubresponseField() { "builder.addAllEntries(request.getEntriesList());\n", "}\n", "}\n", + "\n", "@Override\n", "public WriteLogEntriesRequest build() {\n", "return builder.build();\n", "}\n", + "\n", "};\n", "}\n", + "\n", "@Override\n", - "public void splitResponse(WriteLogEntriesResponse batchResponse, ", - "Collection> batch) {\n", + "public void splitResponse(WriteLogEntriesResponse batchResponse, Collection> batch) {\n", "for (BatchedRequestIssuer responder : batch) {\n", "WriteLogEntriesResponse response = WriteLogEntriesResponse.newBuilder().build();\n", "responder.setResponse(response);\n", "}\n", "}\n", + "\n", "@Override\n", - "public void splitException(", - "Throwable throwable, ", - "Collection> batch) {\n", + "public void splitException(Throwable throwable, Collection> batch) {\n", "for (BatchedRequestIssuer responder : batch) {\n", "responder.setException(throwable);\n", "}\n", "}\n", + "\n", "@Override\n", "public long countElements(WriteLogEntriesRequest request) {\n", "return request.getEntriesCount();\n", "}\n", + "\n", "@Override\n", "public long countBytes(WriteLogEntriesRequest request) {\n", "return request.getSerializedSize();\n", "}\n", + "\n", "}"); assertEquals(expected, writerVisitor.write()); diff --git a/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceCallableFactoryClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceCallableFactoryClassComposerTest.java index 2498618e01..c935ceca41 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceCallableFactoryClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceCallableFactoryClassComposerTest.java @@ -85,6 +85,7 @@ public void generateServiceClasses() { + "\n" + "@Generated(\"by gapic-generator\")\n" + "public class GrpcEchoCallableFactory implements GrpcStubCallableFactory {\n" + + "\n" + " @Override\n" + " public UnaryCallable" + " createUnaryCallable(\n" diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java index 91034d9792..6490109316 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java @@ -89,6 +89,7 @@ public void generateServiceClasses() { + "\n" + "@Generated(\"by gapic-generator-java\")\n" + "public class EchoSettings extends ClientSettings {\n" + + "\n" + " public UnaryCallSettings echoSettings() {\n" + " return ((EchoStubSettings) getStubSettings()).echoSettings();\n" + " }\n" @@ -186,6 +187,7 @@ public void generateServiceClasses() { + "\n" + " public static class Builder extends ClientSettings.Builder" + " {\n" + + "\n" + " protected Builder() throws IOException {\n" + " this(((ClientContext) null));\n" + " }\n" diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubClassComposerTest.java index f494c7afb4..54d9ef5051 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubClassComposerTest.java @@ -84,6 +84,7 @@ public void generateServiceClasses() { + "\n" + "@Generated(\"by gapic-generator\")\n" + "public abstract class EchoStub implements BackgroundResource {\n" + + "\n" + " public OperationsStub getOperationsStub() {\n" + " throw new UnsupportedOperationException(\"Not implemented:" + " getOperationsStub()\");\n" 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 52b025ead1..1415ba3fd7 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 @@ -228,12 +228,15 @@ private static List parseServices( + "import javax.annotation.Generated;\n" + "import org.threeten.bp.Duration;\n" + "\n" + + "// AUTO-GENERATED DOCUMENTATION AND CLASS.\n" + "@BetaApi\n" + "@Generated(\"by gapic-generator-java\")\n" + "public class EchoStubSettings extends StubSettings {\n" + + " /** The default scopes of the service. */\n" + " private static final ImmutableList DEFAULT_SERVICE_SCOPES =\n" + " " + " ImmutableList.builder().add(\"https://www.googleapis.com/auth/cloud-platform\").build();\n" + + "\n" + " private final UnaryCallSettings echoSettings;\n" + " private final ServerStreamingCallSettings" + " expandSettings;\n" @@ -310,42 +313,51 @@ private static List parseServices( + " }\n" + " };\n" + "\n" + + " /** Returns the object with the settings used for calls to echo. */\n" + " public UnaryCallSettings echoSettings() {\n" + " return echoSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to expand. */\n" + " public ServerStreamingCallSettings expandSettings()" + " {\n" + " return expandSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to collect. */\n" + " public StreamingCallSettings collectSettings() {\n" + " return collectSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to chat. */\n" + " public StreamingCallSettings chatSettings() {\n" + " return chatSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to chatAgain. */\n" + " public StreamingCallSettings chatAgainSettings() {\n" + " return chatAgainSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to pagedExpand. */\n" + " public PagedCallSettings\n" + " pagedExpandSettings() {\n" + " return pagedExpandSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to wait. */\n" + " public UnaryCallSettings waitSettings() {\n" + " return waitSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to wait. */\n" + " public OperationCallSettings" + " waitOperationSettings() {\n" + " return waitOperationSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to block. */\n" + " public UnaryCallSettings blockSettings() {\n" + " return blockSettings;\n" + " }\n" @@ -364,25 +376,30 @@ private static List parseServices( + " getTransportChannelProvider().getTransportName()));\n" + " }\n" + "\n" + + " /** Returns a builder for the default ExecutorProvider for this service. */\n" + " public static InstantiatingExecutorProvider.Builder" + " defaultExecutorProviderBuilder() {\n" + " return InstantiatingExecutorProvider.newBuilder();\n" + " }\n" + "\n" + + " /** Returns the default service endpoint. */\n" + " public static String getDefaultEndpoint() {\n" + " return \"localhost:7469\";\n" + " }\n" + "\n" + + " /** Returns the default service scopes. */\n" + " public static List getDefaultServiceScopes() {\n" + " return DEFAULT_SERVICE_SCOPES;\n" + " }\n" + "\n" + + " /** Returns a builder for the default credentials for this service. */\n" + " public static GoogleCredentialsProvider.Builder defaultCredentialsProviderBuilder()" + " {\n" + " return" + " GoogleCredentialsProvider.newBuilder().setScopesToApply(DEFAULT_SERVICE_SCOPES);\n" + " }\n" + "\n" + + " /** Returns a builder for the default ChannelProvider for this service. */\n" + " public static InstantiatingGrpcChannelProvider.Builder" + " defaultGrpcTransportProviderBuilder() {\n" + " return InstantiatingGrpcChannelProvider.newBuilder()\n" @@ -405,14 +422,17 @@ private static List parseServices( + " GaxGrpcProperties.getGrpcVersion());\n" + " }\n" + "\n" + + " /** Returns a new builder for this class. */\n" + " public static Builder newBuilder() {\n" + " return Builder.createDefault();\n" + " }\n" + "\n" + + " /** Returns a new builder for this class. */\n" + " public static Builder newBuilder(ClientContext clientContext) {\n" + " return new Builder(clientContext);\n" + " }\n" + "\n" + + " /** Returns a builder containing all the values of this settings class. */\n" + " public Builder toBuilder() {\n" + " return new Builder(this);\n" + " }\n" @@ -430,6 +450,7 @@ private static List parseServices( + " blockSettings = settingsBuilder.blockSettings().build();\n" + " }\n" + "\n" + + " /** Builder for EchoStubSettings. */\n" + " public static class Builder extends StubSettings.Builder {\n" + " private final ImmutableList>" + " unaryMethodSettingsBuilders;\n" @@ -603,6 +624,14 @@ private static List parseServices( + " return builder;\n" + " }\n" + "\n" + + " // NEXT_MAJOR_VER: remove 'throws Exception'.\n" + + " /**\n" + + " * Applies the given settings updater function to all of the unary API methods in" + + " this service.\n" + + " *\n" + + " *

Note: This method does not support applying settings to streaming" + + " methods.\n" + + " */\n" + " public Builder applyToAllUnaryMethods(\n" + " ApiFunction, Void> settingsUpdater) throws" + " Exception {\n" @@ -615,40 +644,48 @@ private static List parseServices( + " return unaryMethodSettingsBuilders;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to echo. */\n" + " public UnaryCallSettings.Builder echoSettings() {\n" + " return echoSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to expand. */\n" + " public ServerStreamingCallSettings.Builder" + " expandSettings() {\n" + " return expandSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to collect. */\n" + " public StreamingCallSettings.Builder collectSettings()" + " {\n" + " return collectSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to chat. */\n" + " public StreamingCallSettings.Builder chatSettings()" + " {\n" + " return chatSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to chatAgain. */\n" + " public StreamingCallSettings.Builder" + " chatAgainSettings() {\n" + " return chatAgainSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to pagedExpand. */\n" + " public PagedCallSettings.Builder<\n" + " PagedExpandRequest, PagedExpandResponse, PagedExpandPagedResponse>\n" + " pagedExpandSettings() {\n" + " return pagedExpandSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to wait. */\n" + " public UnaryCallSettings.Builder waitSettings() {\n" + " return waitSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to wait. */\n" + " @BetaApi(\n" + " \"The surface for use by generated code is not stable yet and may change in" + " the future.\")\n" @@ -657,6 +694,7 @@ private static List parseServices( + " return waitOperationSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to block. */\n" + " public UnaryCallSettings.Builder blockSettings() {\n" + " return blockSettings;\n" + " }\n" @@ -730,10 +768,12 @@ private static List parseServices( + "import javax.annotation.Generated;\n" + "import org.threeten.bp.Duration;\n" + "\n" + + "// AUTO-GENERATED DOCUMENTATION AND CLASS.\n" + "@BetaApi\n" + "@Generated(\"by gapic-generator-java\")\n" + "public class LoggingServiceV2StubSettings extends" + " StubSettings {\n" + + " /** The default scopes of the service. */\n" + " private static final ImmutableList DEFAULT_SERVICE_SCOPES =\n" + " ImmutableList.builder()\n" + " .add(\"https://www.googleapis.com/auth/cloud-platform\")\n" @@ -742,6 +782,7 @@ private static List parseServices( + " .add(\"https://www.googleapis.com/auth/logging.read\")\n" + " .add(\"https://www.googleapis.com/auth/logging.write\")\n" + " .build();\n" + + "\n" + " private final UnaryCallSettings deleteLogSettings;\n" + " private final BatchingCallSettings\n" @@ -1033,15 +1074,18 @@ private static List parseServices( + " }\n" + " };\n" + "\n" + + " /** Returns the object with the settings used for calls to deleteLog. */\n" + " public UnaryCallSettings deleteLogSettings() {\n" + " return deleteLogSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to writeLogEntries. */\n" + " public BatchingCallSettings\n" + " writeLogEntriesSettings() {\n" + " return writeLogEntriesSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to listLogEntries. */\n" + " public PagedCallSettings<\n" + " ListLogEntriesRequest, ListLogEntriesResponse," + " ListLogEntriesPagedResponse>\n" @@ -1049,6 +1093,8 @@ private static List parseServices( + " return listLogEntriesSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to" + + " listMonitoredResourceDescriptors. */\n" + " public PagedCallSettings<\n" + " ListMonitoredResourceDescriptorsRequest,\n" + " ListMonitoredResourceDescriptorsResponse,\n" @@ -1057,6 +1103,7 @@ private static List parseServices( + " return listMonitoredResourceDescriptorsSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to listLogs. */\n" + " public PagedCallSettings\n" + " listLogsSettings() {\n" @@ -1077,25 +1124,30 @@ private static List parseServices( + " getTransportChannelProvider().getTransportName()));\n" + " }\n" + "\n" + + " /** Returns a builder for the default ExecutorProvider for this service. */\n" + " public static InstantiatingExecutorProvider.Builder" + " defaultExecutorProviderBuilder() {\n" + " return InstantiatingExecutorProvider.newBuilder();\n" + " }\n" + "\n" + + " /** Returns the default service endpoint. */\n" + " public static String getDefaultEndpoint() {\n" + " return \"logging.googleapis.com:443\";\n" + " }\n" + "\n" + + " /** Returns the default service scopes. */\n" + " public static List getDefaultServiceScopes() {\n" + " return DEFAULT_SERVICE_SCOPES;\n" + " }\n" + "\n" + + " /** Returns a builder for the default credentials for this service. */\n" + " public static GoogleCredentialsProvider.Builder defaultCredentialsProviderBuilder()" + " {\n" + " return" + " GoogleCredentialsProvider.newBuilder().setScopesToApply(DEFAULT_SERVICE_SCOPES);\n" + " }\n" + "\n" + + " /** Returns a builder for the default ChannelProvider for this service. */\n" + " public static InstantiatingGrpcChannelProvider.Builder" + " defaultGrpcTransportProviderBuilder() {\n" + " return InstantiatingGrpcChannelProvider.newBuilder()\n" @@ -1119,14 +1171,17 @@ private static List parseServices( + " GaxGrpcProperties.getGrpcVersion());\n" + " }\n" + "\n" + + " /** Returns a new builder for this class. */\n" + " public static Builder newBuilder() {\n" + " return Builder.createDefault();\n" + " }\n" + "\n" + + " /** Returns a new builder for this class. */\n" + " public static Builder newBuilder(ClientContext clientContext) {\n" + " return new Builder(clientContext);\n" + " }\n" + "\n" + + " /** Returns a builder containing all the values of this settings class. */\n" + " public Builder toBuilder() {\n" + " return new Builder(this);\n" + " }\n" @@ -1142,6 +1197,7 @@ private static List parseServices( + " listLogsSettings = settingsBuilder.listLogsSettings().build();\n" + " }\n" + "\n" + + " /** Builder for LoggingServiceV2StubSettings. */\n" + " public static class Builder extends StubSettings.Builder {\n" + " private final ImmutableList>" @@ -1302,6 +1358,14 @@ private static List parseServices( + " return builder;\n" + " }\n" + "\n" + + " // NEXT_MAJOR_VER: remove 'throws Exception'.\n" + + " /**\n" + + " * Applies the given settings updater function to all of the unary API methods in" + + " this service.\n" + + " *\n" + + " *

Note: This method does not support applying settings to streaming" + + " methods.\n" + + " */\n" + " public Builder applyToAllUnaryMethods(\n" + " ApiFunction, Void> settingsUpdater) throws" + " Exception {\n" @@ -1314,16 +1378,19 @@ private static List parseServices( + " return unaryMethodSettingsBuilders;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to deleteLog. */\n" + " public UnaryCallSettings.Builder deleteLogSettings() {\n" + " return deleteLogSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to writeLogEntries. */\n" + " public BatchingCallSettings.Builder\n" + " writeLogEntriesSettings() {\n" + " return writeLogEntriesSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to listLogEntries. */\n" + " public PagedCallSettings.Builder<\n" + " ListLogEntriesRequest, ListLogEntriesResponse," + " ListLogEntriesPagedResponse>\n" @@ -1331,6 +1398,8 @@ private static List parseServices( + " return listLogEntriesSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to" + + " listMonitoredResourceDescriptors. */\n" + " public PagedCallSettings.Builder<\n" + " ListMonitoredResourceDescriptorsRequest,\n" + " ListMonitoredResourceDescriptorsResponse,\n" @@ -1339,6 +1408,7 @@ private static List parseServices( + " return listMonitoredResourceDescriptorsSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to listLogs. */\n" + " public PagedCallSettings.Builder\n" + " listLogsSettings() {\n" @@ -1417,14 +1487,17 @@ private static List parseServices( + "import javax.annotation.Generated;\n" + "import org.threeten.bp.Duration;\n" + "\n" + + "// AUTO-GENERATED DOCUMENTATION AND CLASS.\n" + "@BetaApi\n" + "@Generated(\"by gapic-generator-java\")\n" + "public class PublisherStubSettings extends StubSettings {\n" + + " /** The default scopes of the service. */\n" + " private static final ImmutableList DEFAULT_SERVICE_SCOPES =\n" + " ImmutableList.builder()\n" + " .add(\"https://www.googleapis.com/auth/cloud-platform\")\n" + " .add(\"https://www.googleapis.com/auth/pubsub\")\n" + " .build();\n" + + "\n" + " private final UnaryCallSettings createTopicSettings;\n" + " private final UnaryCallSettings updateTopicSettings;\n" + " private final BatchingCallSettings" @@ -1714,28 +1787,35 @@ private static List parseServices( + " }\n" + " };\n" + "\n" + + " /** Returns the object with the settings used for calls to createTopic. */\n" + " public UnaryCallSettings createTopicSettings() {\n" + " return createTopicSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to updateTopic. */\n" + " public UnaryCallSettings updateTopicSettings() {\n" + " return updateTopicSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to publish. */\n" + " public BatchingCallSettings publishSettings() {\n" + " return publishSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to getTopic. */\n" + " public UnaryCallSettings getTopicSettings() {\n" + " return getTopicSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to listTopics. */\n" + " public PagedCallSettings\n" + " listTopicsSettings() {\n" + " return listTopicsSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to listTopicSubscriptions." + + " */\n" + " public PagedCallSettings<\n" + " ListTopicSubscriptionsRequest,\n" + " ListTopicSubscriptionsResponse,\n" @@ -1744,6 +1824,7 @@ private static List parseServices( + " return listTopicSubscriptionsSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to listTopicSnapshots. */\n" + " public PagedCallSettings<\n" + " ListTopicSnapshotsRequest, ListTopicSnapshotsResponse," + " ListTopicSnapshotsPagedResponse>\n" @@ -1751,10 +1832,12 @@ private static List parseServices( + " return listTopicSnapshotsSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to deleteTopic. */\n" + " public UnaryCallSettings deleteTopicSettings() {\n" + " return deleteTopicSettings;\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to detachSubscription. */\n" + " public UnaryCallSettings\n" + " detachSubscriptionSettings() {\n" + " return detachSubscriptionSettings;\n" @@ -1774,25 +1857,30 @@ private static List parseServices( + " getTransportChannelProvider().getTransportName()));\n" + " }\n" + "\n" + + " /** Returns a builder for the default ExecutorProvider for this service. */\n" + " public static InstantiatingExecutorProvider.Builder" + " defaultExecutorProviderBuilder() {\n" + " return InstantiatingExecutorProvider.newBuilder();\n" + " }\n" + "\n" + + " /** Returns the default service endpoint. */\n" + " public static String getDefaultEndpoint() {\n" + " return \"pubsub.googleapis.com:443\";\n" + " }\n" + "\n" + + " /** Returns the default service scopes. */\n" + " public static List getDefaultServiceScopes() {\n" + " return DEFAULT_SERVICE_SCOPES;\n" + " }\n" + "\n" + + " /** Returns a builder for the default credentials for this service. */\n" + " public static GoogleCredentialsProvider.Builder defaultCredentialsProviderBuilder()" + " {\n" + " return" + " GoogleCredentialsProvider.newBuilder().setScopesToApply(DEFAULT_SERVICE_SCOPES);\n" + " }\n" + "\n" + + " /** Returns a builder for the default ChannelProvider for this service. */\n" + " public static InstantiatingGrpcChannelProvider.Builder" + " defaultGrpcTransportProviderBuilder() {\n" + " return InstantiatingGrpcChannelProvider.newBuilder()\n" @@ -1815,14 +1903,17 @@ private static List parseServices( + " GaxGrpcProperties.getGrpcVersion());\n" + " }\n" + "\n" + + " /** Returns a new builder for this class. */\n" + " public static Builder newBuilder() {\n" + " return Builder.createDefault();\n" + " }\n" + "\n" + + " /** Returns a new builder for this class. */\n" + " public static Builder newBuilder(ClientContext clientContext) {\n" + " return new Builder(clientContext);\n" + " }\n" + "\n" + + " /** Returns a builder containing all the values of this settings class. */\n" + " public Builder toBuilder() {\n" + " return new Builder(this);\n" + " }\n" @@ -1843,6 +1934,7 @@ private static List parseServices( + " settingsBuilder.detachSubscriptionSettings().build();\n" + " }\n" + "\n" + + " /** Builder for PublisherStubSettings. */\n" + " public static class Builder extends StubSettings.Builder" + " {\n" + " private final ImmutableList>" @@ -2083,6 +2175,14 @@ private static List parseServices( + " return builder;\n" + " }\n" + "\n" + + " // NEXT_MAJOR_VER: remove 'throws Exception'.\n" + + " /**\n" + + " * Applies the given settings updater function to all of the unary API methods in" + + " this service.\n" + + " *\n" + + " *

Note: This method does not support applying settings to streaming" + + " methods.\n" + + " */\n" + " public Builder applyToAllUnaryMethods(\n" + " ApiFunction, Void> settingsUpdater) throws" + " Exception {\n" @@ -2095,30 +2195,37 @@ private static List parseServices( + " return unaryMethodSettingsBuilders;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to createTopic. */\n" + " public UnaryCallSettings.Builder createTopicSettings() {\n" + " return createTopicSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to updateTopic. */\n" + " public UnaryCallSettings.Builder updateTopicSettings()" + " {\n" + " return updateTopicSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to publish. */\n" + " public BatchingCallSettings.Builder" + " publishSettings() {\n" + " return publishSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to getTopic. */\n" + " public UnaryCallSettings.Builder getTopicSettings() {\n" + " return getTopicSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to listTopics. */\n" + " public PagedCallSettings.Builder\n" + " listTopicsSettings() {\n" + " return listTopicsSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to" + + " listTopicSubscriptions. */\n" + " public PagedCallSettings.Builder<\n" + " ListTopicSubscriptionsRequest,\n" + " ListTopicSubscriptionsResponse,\n" @@ -2127,6 +2234,8 @@ private static List parseServices( + " return listTopicSubscriptionsSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to listTopicSnapshots." + + " */\n" + " public PagedCallSettings.Builder<\n" + " ListTopicSnapshotsRequest, ListTopicSnapshotsResponse," + " ListTopicSnapshotsPagedResponse>\n" @@ -2134,11 +2243,14 @@ private static List parseServices( + " return listTopicSnapshotsSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to deleteTopic. */\n" + " public UnaryCallSettings.Builder deleteTopicSettings()" + " {\n" + " return deleteTopicSettings;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to detachSubscription." + + " */\n" + " public UnaryCallSettings.Builder\n" + " detachSubscriptionSettings() {\n" From 5fa910c609bb19093fe7dcc6103e24396c306c7f Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 3 Sep 2020 14:26:19 -0700 Subject: [PATCH 11/15] feat: add class Javadoc to ServiceStubSettings --- .../generator/engine/ast/JavaDocComment.java | 5 ++ .../ServiceStubSettingsClassComposer.java | 12 +++- .../ServiceStubSettingsCommentComposer.java | 62 +++++++++++++++++++ .../ServiceStubSettingsClassComposerTest.java | 62 +++++++++++++++++++ 4 files changed, 140 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/JavaDocComment.java b/src/main/java/com/google/api/generator/engine/ast/JavaDocComment.java index 5e0a4665e8..5ab53ca646 100644 --- a/src/main/java/com/google/api/generator/engine/ast/JavaDocComment.java +++ b/src/main/java/com/google/api/generator/engine/ast/JavaDocComment.java @@ -71,6 +71,11 @@ public Builder addParam(String name, String description) { return this; } + public Builder addUnescapedComment(String comment) { + componentsList.add(comment); + return this; + } + public Builder addComment(String comment) { componentsList.add(HtmlEscaper.escaper(comment)); return this; 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 e7b4b87072..26dfc9508b 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 @@ -56,6 +56,7 @@ import com.google.api.generator.engine.ast.AssignmentExpr; import com.google.api.generator.engine.ast.CastExpr; import com.google.api.generator.engine.ast.ClassDefinition; +import com.google.api.generator.engine.ast.CommentStatement; import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; @@ -163,7 +164,7 @@ public GapicClass generate( ClassDefinition classDef = ClassDefinition.builder() .setPackageString(pakkage) - .setHeaderCommentStatements(CommentComposer.AUTO_GENERATED_CLASS_COMMENT) + .setHeaderCommentStatements(createClassHeaderComments(service)) .setAnnotations(createClassAnnotations()) .setScope(ScopeNode.PUBLIC) .setName(className) @@ -187,6 +188,15 @@ private static List createClassAnnotations() { .build()); } + private static List createClassHeaderComments(Service service) { + Optional methodOpt = + service.methods().isEmpty() ? Optional.empty() : Optional.of(service.methods().get(0)); + return Arrays.asList( + CommentComposer.AUTO_GENERATED_CLASS_COMMENT, + ServiceStubSettingsCommentComposer.createClassHeaderComment( + getThisClassName(service.name()), service.defaultHost(), methodOpt)); + } + private static TypeNode createExtendsType(Service service, Map types) { TypeNode thisClassType = types.get(getThisClassName(service.name())); return TypeNode.withReference( diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsCommentComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsCommentComposer.java index 659c37d175..fb0f778b7e 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsCommentComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsCommentComposer.java @@ -17,17 +17,42 @@ import com.google.api.generator.engine.ast.CommentStatement; import com.google.api.generator.engine.ast.JavaDocComment; import com.google.api.generator.engine.ast.LineComment; +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.Arrays; import java.util.List; +import java.util.Optional; import java.util.stream.Collectors; class ServiceStubSettingsCommentComposer { + private static final String COLON = ":"; + private static final String BUILDER_CLASS_DOC_PATTERN = "Builder for %s."; private static final String CALL_SETTINGS_METHOD_DOC_PATTERN = "Returns the object with the settings used for calls to %s."; private static final String CALL_SETTINGS_BUILDER_METHOD_DOC_PATTERN = "Returns the builder for the settings used for calls to %s."; + // Class header patterns. + private static final String CLASS_HEADER_SUMMARY_PATTERN = + "Settings class to configure an instance of {@link %s}."; + private static final String CLASS_HEADER_DEFAULT_ADDRESS_PORT_PATTERN = + "The default service address (%s) and default port (%d) are used."; + private static final String CLASS_HEADER_SAMPLE_CODE_PATTERN = + "For example, to set the total timeout of %s to 30 seconds:"; + + private static final String CLASS_HEADER_BUILDER_DESCRIPTION = + "The builder of this class is recursive, so contained classes are themselves builders. When" + + " build() is called, the tree of builders is called to create the complete settings" + + " object."; + private static final String CLASS_HEADER_DEFAULTS_DESCRIPTION = + "The default instance has everything set to sensible defaults:"; + private static final String CLASS_HEADER_DEFAULTS_CREDENTIALS_DESCRIPTION = + "Credentials are acquired automatically through Application Default Credentials."; + private static final String CLASS_HEADER_DEFAULTS_RETRIES_DESCRIPTION = + "Retries are configured for idempotent methods but not for non-idempotent methods."; + static final CommentStatement DEFAULT_SCOPES_COMMENT = toSimpleComment("The default scopes of the service."); @@ -76,6 +101,43 @@ static CommentStatement createCallSettingsBuilderGetterComment(String javaMethod return toSimpleComment(String.format(CALL_SETTINGS_BUILDER_METHOD_DOC_PATTERN, javaMethodName)); } + static CommentStatement createClassHeaderComment( + String className, String defaultHost, Optional methodOpt) { + // Split default address and port. + int colonIndex = defaultHost.indexOf(COLON); + Preconditions.checkState( + colonIndex > 0 && colonIndex < defaultHost.length() - 1, + String.format( + "No valid address and port found for %s, expected a string formatted like" + + " localhost:8888", + defaultHost)); + String defaultAddress = defaultHost.substring(0, colonIndex); + int defaultPort = Integer.parseInt(defaultHost.substring(colonIndex + 1)); + + JavaDocComment.Builder javaDocCommentBuilder = + JavaDocComment.builder() + .addUnescapedComment(String.format(CLASS_HEADER_SUMMARY_PATTERN, className)) + .addParagraph(CLASS_HEADER_DEFAULTS_DESCRIPTION) + .addUnorderedList( + Arrays.asList( + String.format( + CLASS_HEADER_DEFAULT_ADDRESS_PORT_PATTERN, defaultAddress, defaultPort), + CLASS_HEADER_DEFAULTS_CREDENTIALS_DESCRIPTION, + CLASS_HEADER_DEFAULTS_RETRIES_DESCRIPTION)) + .addParagraph(CLASS_HEADER_BUILDER_DESCRIPTION); + + if (methodOpt.isPresent()) { + javaDocCommentBuilder = + javaDocCommentBuilder.addParagraph( + String.format( + CLASS_HEADER_SAMPLE_CODE_PATTERN, + JavaStyle.toLowerCamelCase(methodOpt.get().name()))); + // TODO(summerji): Add sample code here. + } + + return CommentStatement.withComment(javaDocCommentBuilder.build()); + } + private static CommentStatement toSimpleComment(String comment) { return CommentStatement.withComment(JavaDocComment.withComment(comment)); } 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 1415ba3fd7..8413959746 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 @@ -229,6 +229,26 @@ private static List parseServices( + "import org.threeten.bp.Duration;\n" + "\n" + "// AUTO-GENERATED DOCUMENTATION AND CLASS.\n" + + "/**\n" + + " * Settings class to configure an instance of {@link EchoStubSettings}.\n" + + " *\n" + + " *

The default instance has everything set to sensible defaults:\n" + + " *\n" + + " *

    \n" + + " *
  • The default service address (localhost) and default port (7469) are used.\n" + + " *
  • Credentials are acquired automatically through Application Default" + + " Credentials.\n" + + " *
  • Retries are configured for idempotent methods but not for non-idempotent" + + " methods.\n" + + " *
\n" + + " *\n" + + " *

The builder of this class is recursive, so contained classes are themselves" + + " builders. When\n" + + " * build() is called, the tree of builders is called to create the complete settings" + + " object.\n" + + " *\n" + + " *

For example, to set the total timeout of echo to 30 seconds:\n" + + " */\n" + "@BetaApi\n" + "@Generated(\"by gapic-generator-java\")\n" + "public class EchoStubSettings extends StubSettings {\n" @@ -769,6 +789,27 @@ private static List parseServices( + "import org.threeten.bp.Duration;\n" + "\n" + "// AUTO-GENERATED DOCUMENTATION AND CLASS.\n" + + "/**\n" + + " * Settings class to configure an instance of {@link LoggingServiceV2StubSettings}.\n" + + " *\n" + + " *

The default instance has everything set to sensible defaults:\n" + + " *\n" + + " *

    \n" + + " *
  • The default service address (logging.googleapis.com) and default port (443)" + + " are used.\n" + + " *
  • Credentials are acquired automatically through Application Default" + + " Credentials.\n" + + " *
  • Retries are configured for idempotent methods but not for non-idempotent" + + " methods.\n" + + " *
\n" + + " *\n" + + " *

The builder of this class is recursive, so contained classes are themselves" + + " builders. When\n" + + " * build() is called, the tree of builders is called to create the complete settings" + + " object.\n" + + " *\n" + + " *

For example, to set the total timeout of deleteLog to 30 seconds:\n" + + " */\n" + "@BetaApi\n" + "@Generated(\"by gapic-generator-java\")\n" + "public class LoggingServiceV2StubSettings extends" @@ -1488,6 +1529,27 @@ private static List parseServices( + "import org.threeten.bp.Duration;\n" + "\n" + "// AUTO-GENERATED DOCUMENTATION AND CLASS.\n" + + "/**\n" + + " * Settings class to configure an instance of {@link PublisherStubSettings}.\n" + + " *\n" + + " *

The default instance has everything set to sensible defaults:\n" + + " *\n" + + " *

    \n" + + " *
  • The default service address (pubsub.googleapis.com) and default port (443)" + + " are used.\n" + + " *
  • Credentials are acquired automatically through Application Default" + + " Credentials.\n" + + " *
  • Retries are configured for idempotent methods but not for non-idempotent" + + " methods.\n" + + " *
\n" + + " *\n" + + " *

The builder of this class is recursive, so contained classes are themselves" + + " builders. When\n" + + " * build() is called, the tree of builders is called to create the complete settings" + + " object.\n" + + " *\n" + + " *

For example, to set the total timeout of createTopic to 30 seconds:\n" + + " */\n" + "@BetaApi\n" + "@Generated(\"by gapic-generator-java\")\n" + "public class PublisherStubSettings extends StubSettings {\n" From 6f570e3b5bb17f11ef097728120cd4735cf3ac90 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 3 Sep 2020 14:26:19 -0700 Subject: [PATCH 12/15] feat: add class Javadoc to ServiceStubSettings --- .../generator/engine/ast/JavaDocComment.java | 5 ++ .../ServiceStubSettingsClassComposer.java | 12 +++- .../ServiceStubSettingsCommentComposer.java | 62 +++++++++++++++++++ .../ServiceStubSettingsClassComposerTest.java | 62 +++++++++++++++++++ 4 files changed, 140 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/JavaDocComment.java b/src/main/java/com/google/api/generator/engine/ast/JavaDocComment.java index 5e0a4665e8..5ab53ca646 100644 --- a/src/main/java/com/google/api/generator/engine/ast/JavaDocComment.java +++ b/src/main/java/com/google/api/generator/engine/ast/JavaDocComment.java @@ -71,6 +71,11 @@ public Builder addParam(String name, String description) { return this; } + public Builder addUnescapedComment(String comment) { + componentsList.add(comment); + return this; + } + public Builder addComment(String comment) { componentsList.add(HtmlEscaper.escaper(comment)); return this; 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 e7b4b87072..713dd68753 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 @@ -56,6 +56,7 @@ import com.google.api.generator.engine.ast.AssignmentExpr; import com.google.api.generator.engine.ast.CastExpr; import com.google.api.generator.engine.ast.ClassDefinition; +import com.google.api.generator.engine.ast.CommentStatement; import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; @@ -163,7 +164,7 @@ public GapicClass generate( ClassDefinition classDef = ClassDefinition.builder() .setPackageString(pakkage) - .setHeaderCommentStatements(CommentComposer.AUTO_GENERATED_CLASS_COMMENT) + .setHeaderCommentStatements(createClassHeaderComments(service)) .setAnnotations(createClassAnnotations()) .setScope(ScopeNode.PUBLIC) .setName(className) @@ -187,6 +188,15 @@ private static List createClassAnnotations() { .build()); } + private static List createClassHeaderComments(Service service) { + Optional methodOpt = + service.methods().isEmpty() ? Optional.empty() : Optional.of(service.methods().get(0)); + return Arrays.asList( + CommentComposer.AUTO_GENERATED_CLASS_COMMENT, + ServiceStubSettingsCommentComposer.createClassHeaderComment( + String.format(STUB_PATTERN, service.name()), service.defaultHost(), methodOpt)); + } + private static TypeNode createExtendsType(Service service, Map types) { TypeNode thisClassType = types.get(getThisClassName(service.name())); return TypeNode.withReference( diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsCommentComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsCommentComposer.java index 659c37d175..b272e90365 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsCommentComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsCommentComposer.java @@ -17,17 +17,42 @@ import com.google.api.generator.engine.ast.CommentStatement; import com.google.api.generator.engine.ast.JavaDocComment; import com.google.api.generator.engine.ast.LineComment; +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.Arrays; import java.util.List; +import java.util.Optional; import java.util.stream.Collectors; class ServiceStubSettingsCommentComposer { + private static final String COLON = ":"; + private static final String BUILDER_CLASS_DOC_PATTERN = "Builder for %s."; private static final String CALL_SETTINGS_METHOD_DOC_PATTERN = "Returns the object with the settings used for calls to %s."; private static final String CALL_SETTINGS_BUILDER_METHOD_DOC_PATTERN = "Returns the builder for the settings used for calls to %s."; + // Class header patterns. + private static final String CLASS_HEADER_SUMMARY_PATTERN = + "Settings class to configure an instance of {@link %s}."; + private static final String CLASS_HEADER_DEFAULT_ADDRESS_PORT_PATTERN = + "The default service address (%s) and default port (%d) are used."; + private static final String CLASS_HEADER_SAMPLE_CODE_PATTERN = + "For example, to set the total timeout of %s to 30 seconds:"; + + private static final String CLASS_HEADER_BUILDER_DESCRIPTION = + "The builder of this class is recursive, so contained classes are themselves builders. When" + + " build() is called, the tree of builders is called to create the complete settings" + + " object."; + private static final String CLASS_HEADER_DEFAULTS_DESCRIPTION = + "The default instance has everything set to sensible defaults:"; + private static final String CLASS_HEADER_DEFAULTS_CREDENTIALS_DESCRIPTION = + "Credentials are acquired automatically through Application Default Credentials."; + private static final String CLASS_HEADER_DEFAULTS_RETRIES_DESCRIPTION = + "Retries are configured for idempotent methods but not for non-idempotent methods."; + static final CommentStatement DEFAULT_SCOPES_COMMENT = toSimpleComment("The default scopes of the service."); @@ -76,6 +101,43 @@ static CommentStatement createCallSettingsBuilderGetterComment(String javaMethod return toSimpleComment(String.format(CALL_SETTINGS_BUILDER_METHOD_DOC_PATTERN, javaMethodName)); } + static CommentStatement createClassHeaderComment( + String configuredClassName, String defaultHost, Optional methodOpt) { + // Split default address and port. + int colonIndex = defaultHost.indexOf(COLON); + Preconditions.checkState( + colonIndex > 0 && colonIndex < defaultHost.length() - 1, + String.format( + "No valid address and port found for %s, expected a string formatted like" + + " localhost:8888", + defaultHost)); + String defaultAddress = defaultHost.substring(0, colonIndex); + int defaultPort = Integer.parseInt(defaultHost.substring(colonIndex + 1)); + + JavaDocComment.Builder javaDocCommentBuilder = + JavaDocComment.builder() + .addUnescapedComment(String.format(CLASS_HEADER_SUMMARY_PATTERN, configuredClassName)) + .addParagraph(CLASS_HEADER_DEFAULTS_DESCRIPTION) + .addUnorderedList( + Arrays.asList( + String.format( + CLASS_HEADER_DEFAULT_ADDRESS_PORT_PATTERN, defaultAddress, defaultPort), + CLASS_HEADER_DEFAULTS_CREDENTIALS_DESCRIPTION, + CLASS_HEADER_DEFAULTS_RETRIES_DESCRIPTION)) + .addParagraph(CLASS_HEADER_BUILDER_DESCRIPTION); + + if (methodOpt.isPresent()) { + javaDocCommentBuilder = + javaDocCommentBuilder.addParagraph( + String.format( + CLASS_HEADER_SAMPLE_CODE_PATTERN, + JavaStyle.toLowerCamelCase(methodOpt.get().name()))); + // TODO(summerji): Add sample code here. + } + + return CommentStatement.withComment(javaDocCommentBuilder.build()); + } + private static CommentStatement toSimpleComment(String comment) { return CommentStatement.withComment(JavaDocComment.withComment(comment)); } 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 1415ba3fd7..2003af7eb2 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 @@ -229,6 +229,26 @@ private static List parseServices( + "import org.threeten.bp.Duration;\n" + "\n" + "// AUTO-GENERATED DOCUMENTATION AND CLASS.\n" + + "/**\n" + + " * Settings class to configure an instance of {@link EchoStub}.\n" + + " *\n" + + " *

The default instance has everything set to sensible defaults:\n" + + " *\n" + + " *

    \n" + + " *
  • The default service address (localhost) and default port (7469) are used.\n" + + " *
  • Credentials are acquired automatically through Application Default" + + " Credentials.\n" + + " *
  • Retries are configured for idempotent methods but not for non-idempotent" + + " methods.\n" + + " *
\n" + + " *\n" + + " *

The builder of this class is recursive, so contained classes are themselves" + + " builders. When\n" + + " * build() is called, the tree of builders is called to create the complete settings" + + " object.\n" + + " *\n" + + " *

For example, to set the total timeout of echo to 30 seconds:\n" + + " */\n" + "@BetaApi\n" + "@Generated(\"by gapic-generator-java\")\n" + "public class EchoStubSettings extends StubSettings {\n" @@ -769,6 +789,27 @@ private static List parseServices( + "import org.threeten.bp.Duration;\n" + "\n" + "// AUTO-GENERATED DOCUMENTATION AND CLASS.\n" + + "/**\n" + + " * Settings class to configure an instance of {@link LoggingServiceV2Stub}.\n" + + " *\n" + + " *

The default instance has everything set to sensible defaults:\n" + + " *\n" + + " *

    \n" + + " *
  • The default service address (logging.googleapis.com) and default port (443)" + + " are used.\n" + + " *
  • Credentials are acquired automatically through Application Default" + + " Credentials.\n" + + " *
  • Retries are configured for idempotent methods but not for non-idempotent" + + " methods.\n" + + " *
\n" + + " *\n" + + " *

The builder of this class is recursive, so contained classes are themselves" + + " builders. When\n" + + " * build() is called, the tree of builders is called to create the complete settings" + + " object.\n" + + " *\n" + + " *

For example, to set the total timeout of deleteLog to 30 seconds:\n" + + " */\n" + "@BetaApi\n" + "@Generated(\"by gapic-generator-java\")\n" + "public class LoggingServiceV2StubSettings extends" @@ -1488,6 +1529,27 @@ private static List parseServices( + "import org.threeten.bp.Duration;\n" + "\n" + "// AUTO-GENERATED DOCUMENTATION AND CLASS.\n" + + "/**\n" + + " * Settings class to configure an instance of {@link PublisherStub}.\n" + + " *\n" + + " *

The default instance has everything set to sensible defaults:\n" + + " *\n" + + " *

    \n" + + " *
  • The default service address (pubsub.googleapis.com) and default port (443)" + + " are used.\n" + + " *
  • Credentials are acquired automatically through Application Default" + + " Credentials.\n" + + " *
  • Retries are configured for idempotent methods but not for non-idempotent" + + " methods.\n" + + " *
\n" + + " *\n" + + " *

The builder of this class is recursive, so contained classes are themselves" + + " builders. When\n" + + " * build() is called, the tree of builders is called to create the complete settings" + + " object.\n" + + " *\n" + + " *

For example, to set the total timeout of createTopic to 30 seconds:\n" + + " */\n" + "@BetaApi\n" + "@Generated(\"by gapic-generator-java\")\n" + "public class PublisherStubSettings extends StubSettings {\n" From 467848dce8b2e080a3dff243e293846d798419b8 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 3 Sep 2020 15:20:24 -0700 Subject: [PATCH 13/15] feat: add comments to ServiceSettings --- .../composer/SerttingsCommentComposer.java | 146 ++++++++++++++++++ .../ServiceSettingsClassComposer.java | 133 +++++++++++----- .../ServiceStubSettingsClassComposer.java | 38 ++--- .../ServiceSettingsClassComposerTest.java | 56 +++++++ 4 files changed, 311 insertions(+), 62 deletions(-) create mode 100644 src/main/java/com/google/api/generator/gapic/composer/SerttingsCommentComposer.java diff --git a/src/main/java/com/google/api/generator/gapic/composer/SerttingsCommentComposer.java b/src/main/java/com/google/api/generator/gapic/composer/SerttingsCommentComposer.java new file mode 100644 index 0000000000..e47822c870 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/composer/SerttingsCommentComposer.java @@ -0,0 +1,146 @@ +// 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.generator.engine.ast.CommentStatement; +import com.google.api.generator.engine.ast.JavaDocComment; +import com.google.api.generator.engine.ast.LineComment; +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.Arrays; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; + +class SettingsCommentComposer { + private static final String COLON = ":"; + + private static final String BUILDER_CLASS_DOC_PATTERN = "Builder for %s."; + private static final String CALL_SETTINGS_METHOD_DOC_PATTERN = + "Returns the object with the settings used for calls to %s."; + private static final String CALL_SETTINGS_BUILDER_METHOD_DOC_PATTERN = + "Returns the builder for the settings used for calls to %s."; + + // Class header patterns. + private static final String CLASS_HEADER_SUMMARY_PATTERN = + "Settings class to configure an instance of {@link %s}."; + private static final String CLASS_HEADER_DEFAULT_ADDRESS_PORT_PATTERN = + "The default service address (%s) and default port (%d) are used."; + private static final String CLASS_HEADER_SAMPLE_CODE_PATTERN = + "For example, to set the total timeout of %s to 30 seconds:"; + + private static final String CLASS_HEADER_BUILDER_DESCRIPTION = + "The builder of this class is recursive, so contained classes are themselves builders. When" + + " build() is called, the tree of builders is called to create the complete settings" + + " object."; + private static final String CLASS_HEADER_DEFAULTS_DESCRIPTION = + "The default instance has everything set to sensible defaults:"; + private static final String CLASS_HEADER_DEFAULTS_CREDENTIALS_DESCRIPTION = + "Credentials are acquired automatically through Application Default Credentials."; + private static final String CLASS_HEADER_DEFAULTS_RETRIES_DESCRIPTION = + "Retries are configured for idempotent methods but not for non-idempotent methods."; + + static final CommentStatement DEFAULT_SCOPES_COMMENT = + toSimpleComment("The default scopes of the service."); + + static final CommentStatement DEFAULT_EXECUTOR_PROVIDER_BUILDER_METHOD_COMMENT = + toSimpleComment("Returns a builder for the default ExecutorProvider for this service."); + static final CommentStatement DEFAULT_SERVICE_ENDPOINT_METHOD_COMMENT = + toSimpleComment("Returns the default service endpoint."); + static final CommentStatement DEFAULT_SERVICE_SCOPES_METHOD_COMMENT = + toSimpleComment("Returns the default service scopes."); + + static final CommentStatement DEFAULT_CREDENTIALS_PROVIDER_BUILDER_METHOD_COMMENT = + toSimpleComment("Returns a builder for the default credentials for this service."); + + static final CommentStatement DEFAULT_GRPC_TRANSPORT_PROVIDER_BUILDER_METHOD_COMMENT = + toSimpleComment("Returns a builder for the default ChannelProvider for this service."); + + static final CommentStatement NEW_BUILDER_METHOD_COMMENT = + toSimpleComment("Returns a new builder for this class."); + + static final CommentStatement TO_BUILDER_METHOD_COMMENT = + toSimpleComment("Returns a builder containing all the values of this settings class."); + + static final List APPLY_TO_ALL_UNARY_METHODS_METHOD_COMMENTS = + Arrays.asList( + LineComment.withComment("NEXT_MAJOR_VER: remove 'throws Exception'."), + JavaDocComment.builder() + .addComment( + "Applies the given settings updater function to all of the unary API methods" + + " in this service.") + .addParagraph( + "Note: This method does not support applying settings to streaming methods.") + .build()) + .stream() + .map(c -> CommentStatement.withComment(c)) + .collect(Collectors.toList()); + + static CommentStatement createCallSettingsGetterComment(String javaMethodName) { + return toSimpleComment(String.format(CALL_SETTINGS_METHOD_DOC_PATTERN, javaMethodName)); + } + + static CommentStatement createBuilderClassComment(String outerClassName) { + return toSimpleComment(String.format(BUILDER_CLASS_DOC_PATTERN, outerClassName)); + } + + static CommentStatement createCallSettingsBuilderGetterComment(String javaMethodName) { + return toSimpleComment(String.format(CALL_SETTINGS_BUILDER_METHOD_DOC_PATTERN, javaMethodName)); + } + + static List createClassHeaderComments( + String configuredClassName, String defaultHost, Optional methodOpt) { + // Split default address and port. + int colonIndex = defaultHost.indexOf(COLON); + Preconditions.checkState( + colonIndex > 0 && colonIndex < defaultHost.length() - 1, + String.format( + "No valid address and port found for %s, expected a string formatted like" + + " localhost:8888", + defaultHost)); + String defaultAddress = defaultHost.substring(0, colonIndex); + int defaultPort = Integer.parseInt(defaultHost.substring(colonIndex + 1)); + + JavaDocComment.Builder javaDocCommentBuilder = + JavaDocComment.builder() + .addUnescapedComment(String.format(CLASS_HEADER_SUMMARY_PATTERN, configuredClassName)) + .addParagraph(CLASS_HEADER_DEFAULTS_DESCRIPTION) + .addUnorderedList( + Arrays.asList( + String.format( + CLASS_HEADER_DEFAULT_ADDRESS_PORT_PATTERN, defaultAddress, defaultPort), + CLASS_HEADER_DEFAULTS_CREDENTIALS_DESCRIPTION, + CLASS_HEADER_DEFAULTS_RETRIES_DESCRIPTION)) + .addParagraph(CLASS_HEADER_BUILDER_DESCRIPTION); + + if (methodOpt.isPresent()) { + javaDocCommentBuilder = + javaDocCommentBuilder.addParagraph( + String.format( + CLASS_HEADER_SAMPLE_CODE_PATTERN, + JavaStyle.toLowerCamelCase(methodOpt.get().name()))); + // TODO(summerji): Add sample code here. + } + + return Arrays.asList( + CommentComposer.AUTO_GENERATED_CLASS_COMMENT, + CommentStatement.withComment(javaDocCommentBuilder.build())); + } + + private static CommentStatement toSimpleComment(String comment) { + return CommentStatement.withComment(JavaDocComment.withComment(comment)); + } +} diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java index dd5d08cd2f..070e860fe5 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java @@ -32,6 +32,7 @@ import com.google.api.generator.engine.ast.AnnotationNode; import com.google.api.generator.engine.ast.CastExpr; import com.google.api.generator.engine.ast.ClassDefinition; +import com.google.api.generator.engine.ast.CommentStatement; import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; @@ -61,9 +62,9 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.function.BiFunction; import java.util.function.Function; import java.util.stream.Collectors; @@ -72,10 +73,14 @@ public class ServiceSettingsClassComposer implements ClassComposer { private static final String BUILDER_CLASS_NAME = "Builder"; private static final String CLASS_NAME_PATTERN = "%sSettings"; + private static final String CLIENT_CLASS_NAME_PATTERN = "%sClient"; private static final String CALL_SETTINGS_TYPE_NAME_PATTERN = "%sCallSettings"; private static final String PAGED_RESPONSE_TYPE_NAME_PATTERN = "%sPagedResponse"; private static final String STUB_SETTINGS_PATTERN = "%sStubSettings"; + private static final String OPERATION_SETTINGS_LITERAL = "OperationSettings"; + private static final String SETTINGS_LITERAL = "Settings"; + private static final ServiceSettingsClassComposer INSTANCE = new ServiceSettingsClassComposer(); private static final Map staticTypes = createStaticTypes(); @@ -96,6 +101,7 @@ public GapicClass generate(Service service, Map ignore) { ClassDefinition classDef = ClassDefinition.builder() .setPackageString(pakkage) + .setHeaderCommentStatements(createClassHeaderComments(service)) .setAnnotations(createClassAnnotations()) .setScope(ScopeNode.PUBLIC) .setName(className) @@ -113,6 +119,13 @@ public GapicClass generate(Service service, Map ignore) { return GapicClass.create(kind, classDef); } + private static List createClassHeaderComments(Service service) { + Optional methodOpt = + service.methods().isEmpty() ? Optional.empty() : Optional.of(service.methods().get(0)); + return SettingsCommentComposer.createClassHeaderComments( + getClientClassName(service.name()), service.defaultHost(), methodOpt); + } + private static List createClassAnnotations() { return Arrays.asList( AnnotationNode.builder() @@ -162,6 +175,9 @@ private static List createSettingsGetterMethods( BiFunction methodMakerFn = (retType, methodName) -> MethodDefinition.builder() + .setHeaderCommentStatements( + SettingsCommentComposer.createCallSettingsGetterComment( + getMethodNameFromSettingsVarName(methodName))) .setScope(ScopeNode.PUBLIC) .setReturnType(retType) .setName(methodName) @@ -242,9 +258,7 @@ private static MethodDefinition createCreatorMethod( private static List createDefaultGetterMethods( Service service, Map types) { - Map javaMethodNameToReturnType = createDefaultMethodNamesToTypes(); - - BiFunction methodMakerFn = + BiFunction methodStarterFn = (mName, retType) -> MethodDefinition.builder() .setScope(ScopeNode.PUBLIC) @@ -256,20 +270,59 @@ private static List createDefaultGetterMethods( .setStaticReferenceType(types.get(getStubSettingsClassName(service.name()))) .setMethodName(mName) .setReturnType(retType) - .build()) - .build(); + .build()); + BiFunction methodMakerFn = + (methodDefBuilder, comment) -> methodDefBuilder.setHeaderCommentStatements(comment).build(); + Function typeMakerFn = + c -> TypeNode.withReference(ConcreteReference.withClazz(c)); + List javaMethods = new ArrayList<>(); - javaMethods.addAll( - javaMethodNameToReturnType.entrySet().stream() - .map(e -> methodMakerFn.apply(e.getKey(), e.getValue())) - .collect(Collectors.toList())); javaMethods.add( - methodMakerFn + methodMakerFn.apply( + methodStarterFn.apply( + "defaultExecutorProviderBuilder", + typeMakerFn.apply(InstantiatingExecutorProvider.Builder.class)), + SettingsCommentComposer.DEFAULT_EXECUTOR_PROVIDER_BUILDER_METHOD_COMMENT)); + javaMethods.add( + methodMakerFn.apply( + methodStarterFn.apply("getDefaultEndpoint", TypeNode.STRING), + SettingsCommentComposer.DEFAULT_SERVICE_ENDPOINT_METHOD_COMMENT)); + javaMethods.add( + methodMakerFn.apply( + methodStarterFn.apply( + "getDefaultServiceScopes", + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(List.class) + .setGenerics(Arrays.asList(TypeNode.STRING.reference())) + .build())), + SettingsCommentComposer.DEFAULT_SERVICE_SCOPES_METHOD_COMMENT)); + javaMethods.add( + methodMakerFn.apply( + methodStarterFn.apply( + "defaultCredentialsProviderBuilder", + typeMakerFn.apply(GoogleCredentialsProvider.Builder.class)), + SettingsCommentComposer.DEFAULT_CREDENTIALS_PROVIDER_BUILDER_METHOD_COMMENT)); + javaMethods.add( + methodMakerFn.apply( + methodStarterFn.apply( + "defaultGrpcTransportProviderBuilder", + typeMakerFn.apply(InstantiatingGrpcChannelProvider.Builder.class)), + SettingsCommentComposer.DEFAULT_GRPC_TRANSPORT_PROVIDER_BUILDER_METHOD_COMMENT)); + + javaMethods.add( + methodStarterFn + .apply( + "defaultTransportChannelProvider", + typeMakerFn.apply(TransportChannelProvider.class)) + .build()); + + javaMethods.add( + methodStarterFn .apply( "defaultApiClientHeaderProviderBuilder", TypeNode.withReference( ConcreteReference.withClazz(ApiClientHeaderProvider.Builder.class))) - .toBuilder() .setAnnotations( Arrays.asList( AnnotationNode.builder() @@ -287,6 +340,7 @@ private static List createBuilderHelperMethods( TypeNode builderType = types.get(BUILDER_CLASS_NAME); MethodDefinition newBuilderMethodOne = MethodDefinition.builder() + .setHeaderCommentStatements(SettingsCommentComposer.NEW_BUILDER_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(builderType) @@ -308,6 +362,7 @@ private static List createBuilderHelperMethods( MethodDefinition newBuilderMethodTwo = MethodDefinition.builder() + .setHeaderCommentStatements(SettingsCommentComposer.NEW_BUILDER_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(builderType) @@ -322,6 +377,7 @@ private static List createBuilderHelperMethods( MethodDefinition toBuilderMethod = MethodDefinition.builder() + .setHeaderCommentStatements(SettingsCommentComposer.TO_BUILDER_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setReturnType(builderType) .setName("toBuilder") @@ -340,6 +396,8 @@ private static List createBuilderHelperMethods( private static ClassDefinition createNestedBuilderClass( Service service, Map types) { return ClassDefinition.builder() + .setHeaderCommentStatements( + SettingsCommentComposer.createBuilderClassComment(getThisClassName(service.name()))) .setIsNested(true) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) @@ -531,6 +589,8 @@ private static MethodDefinition createNestedBuilderApplyToAllUnaryMethod( .build(); return MethodDefinition.builder() + .setHeaderCommentStatements( + SettingsCommentComposer.APPLY_TO_ALL_UNARY_METHODS_METHOD_COMMENTS) .setScope(ScopeNode.PUBLIC) .setReturnType(builderType) .setName(javaMethodName) @@ -547,6 +607,9 @@ private static List createNestedBuilderSettingsGetterMethods( BiFunction methodMakerFn = (retType, methodName) -> MethodDefinition.builder() + .setHeaderCommentStatements( + SettingsCommentComposer.createCallSettingsBuilderGetterComment( + getMethodNameFromSettingsVarName(methodName))) .setScope(ScopeNode.PUBLIC) .setReturnType(retType) .setName(methodName) @@ -623,32 +686,6 @@ private static Map createStaticTypes() { c -> TypeNode.withReference(ConcreteReference.withClazz(c)))); } - private static Map createDefaultMethodNamesToTypes() { - Function typeMakerFn = - c -> TypeNode.withReference(ConcreteReference.withClazz(c)); - Map javaMethodNameToReturnType = new LinkedHashMap<>(); - javaMethodNameToReturnType.put( - "defaultExecutorProviderBuilder", - typeMakerFn.apply(InstantiatingExecutorProvider.Builder.class)); - javaMethodNameToReturnType.put("getDefaultEndpoint", TypeNode.STRING); - javaMethodNameToReturnType.put( - "getDefaultServiceScopes", - TypeNode.withReference( - ConcreteReference.builder() - .setClazz(List.class) - .setGenerics(Arrays.asList(TypeNode.STRING.reference())) - .build())); - javaMethodNameToReturnType.put( - "defaultCredentialsProviderBuilder", - typeMakerFn.apply(GoogleCredentialsProvider.Builder.class)); - javaMethodNameToReturnType.put( - "defaultGrpcTransportProviderBuilder", - typeMakerFn.apply(InstantiatingGrpcChannelProvider.Builder.class)); - javaMethodNameToReturnType.put( - "defaultTransportChannelProvider", staticTypes.get("TransportChannelProvider")); - return javaMethodNameToReturnType; - } - private static Map createDynamicTypes(Service service) { Map types = new HashMap<>(); @@ -692,7 +729,7 @@ private static Map createDynamicTypes(Service service) { VaporReference.builder() .setName(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, m.name())) .setPakkage(service.pakkage()) - .setEnclosingClassName(String.format("%sClient", service.name())) + .setEnclosingClassName(getClientClassName(service.name())) .setIsStaticImport(true) .build())))); return types; @@ -777,6 +814,11 @@ private static String getThisClassName(String serviceName) { return String.format(CLASS_NAME_PATTERN, serviceName); } + private static String getClientClassName(String serviceName) { + + return String.format(CLIENT_CLASS_NAME_PATTERN, serviceName); + } + private static String getStubSettingsClassName(String serviceName) { return String.format(STUB_SETTINGS_PATTERN, serviceName); } @@ -789,4 +831,17 @@ private static TypeNode getStubSettingsBuilderType(Service service) { .setEnclosingClassName(getStubSettingsClassName(service.name())) .build()); } + + /** Turns a name like "waitSettings" or "waitOperationSettings" into "wait". */ + private static String getMethodNameFromSettingsVarName(String settingsVarName) { + BiFunction methodNameSubstrFn = + (s, literal) -> s.substring(0, s.length() - literal.length()); + if (settingsVarName.endsWith(OPERATION_SETTINGS_LITERAL)) { + return methodNameSubstrFn.apply(settingsVarName, OPERATION_SETTINGS_LITERAL); + } + if (settingsVarName.endsWith(SETTINGS_LITERAL)) { + return methodNameSubstrFn.apply(settingsVarName, SETTINGS_LITERAL); + } + return settingsVarName; + } } 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 713dd68753..9e80c622c9 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 @@ -191,10 +191,8 @@ private static List createClassAnnotations() { private static List createClassHeaderComments(Service service) { Optional methodOpt = service.methods().isEmpty() ? Optional.empty() : Optional.of(service.methods().get(0)); - return Arrays.asList( - CommentComposer.AUTO_GENERATED_CLASS_COMMENT, - ServiceStubSettingsCommentComposer.createClassHeaderComment( - String.format(STUB_PATTERN, service.name()), service.defaultHost(), methodOpt)); + return SettingsCommentComposer.createClassHeaderComments( + String.format(STUB_PATTERN, service.name()), service.defaultHost(), methodOpt); } private static TypeNode createExtendsType(Service service, Map types) { @@ -248,7 +246,7 @@ private static List createClassStatements( List statements = new ArrayList<>(); // Assign DEFAULT_SERVICE_SCOPES. - statements.add(ServiceStubSettingsCommentComposer.DEFAULT_SCOPES_COMMENT); + statements.add(SettingsCommentComposer.DEFAULT_SCOPES_COMMENT); VariableExpr defaultServiceScopesDeclVarExpr = DEFAULT_SERVICE_SCOPES_VAR_EXPR .toBuilder() @@ -733,7 +731,7 @@ private static List createMethodSettingsGetterMethods( e -> MethodDefinition.builder() .setHeaderCommentStatements( - ServiceStubSettingsCommentComposer.createCallSettingsGetterComment( + SettingsCommentComposer.createCallSettingsGetterComment( getMethodNameFromSettingsVarName(e.getKey()))) .setScope(ScopeNode.PUBLIC) .setReturnType(e.getValue().type()) @@ -830,7 +828,7 @@ private static List createDefaultHelperAndGetterMethods( javaMethods.add( MethodDefinition.builder() .setHeaderCommentStatements( - ServiceStubSettingsCommentComposer.DEFAULT_EXECUTOR_PROVIDER_BUILDER_METHOD_COMMENT) + SettingsCommentComposer.DEFAULT_EXECUTOR_PROVIDER_BUILDER_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(returnType) @@ -848,7 +846,7 @@ private static List createDefaultHelperAndGetterMethods( javaMethods.add( MethodDefinition.builder() .setHeaderCommentStatements( - ServiceStubSettingsCommentComposer.DEFAULT_SERVICE_ENDPOINT_METHOD_COMMENT) + SettingsCommentComposer.DEFAULT_SERVICE_ENDPOINT_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(returnType) @@ -866,7 +864,7 @@ private static List createDefaultHelperAndGetterMethods( javaMethods.add( MethodDefinition.builder() .setHeaderCommentStatements( - ServiceStubSettingsCommentComposer.DEFAULT_SERVICE_SCOPES_METHOD_COMMENT) + SettingsCommentComposer.DEFAULT_SERVICE_SCOPES_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(returnType) @@ -893,8 +891,7 @@ private static List createDefaultHelperAndGetterMethods( javaMethods.add( MethodDefinition.builder() .setHeaderCommentStatements( - ServiceStubSettingsCommentComposer - .DEFAULT_CREDENTIALS_PROVIDER_BUILDER_METHOD_COMMENT) + SettingsCommentComposer.DEFAULT_CREDENTIALS_PROVIDER_BUILDER_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(returnType) @@ -926,8 +923,7 @@ private static List createDefaultHelperAndGetterMethods( javaMethods.add( MethodDefinition.builder() .setHeaderCommentStatements( - ServiceStubSettingsCommentComposer - .DEFAULT_GRPC_TRANSPORT_PROVIDER_BUILDER_METHOD_COMMENT) + SettingsCommentComposer.DEFAULT_GRPC_TRANSPORT_PROVIDER_BUILDER_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(returnType) @@ -1028,8 +1024,7 @@ private static List createBuilderHelperMethods( final TypeNode builderReturnType = types.get(NESTED_BUILDER_CLASS_NAME); javaMethods.add( MethodDefinition.builder() - .setHeaderCommentStatements( - ServiceStubSettingsCommentComposer.NEW_BUILDER_METHOD_COMMENT) + .setHeaderCommentStatements(SettingsCommentComposer.NEW_BUILDER_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(builderReturnType) @@ -1053,8 +1048,7 @@ private static List createBuilderHelperMethods( .build()); javaMethods.add( MethodDefinition.builder() - .setHeaderCommentStatements( - ServiceStubSettingsCommentComposer.NEW_BUILDER_METHOD_COMMENT) + .setHeaderCommentStatements(SettingsCommentComposer.NEW_BUILDER_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setReturnType(builderReturnType) @@ -1066,8 +1060,7 @@ private static List createBuilderHelperMethods( // Create the toBuilder method. javaMethods.add( MethodDefinition.builder() - .setHeaderCommentStatements( - ServiceStubSettingsCommentComposer.TO_BUILDER_METHOD_COMMENT) + .setHeaderCommentStatements(SettingsCommentComposer.TO_BUILDER_METHOD_COMMENT) .setScope(ScopeNode.PUBLIC) .setReturnType(builderReturnType) .setName("toBuilder") @@ -1158,8 +1151,7 @@ private static ClassDefinition createNestedBuilderClass( return ClassDefinition.builder() .setIsNested(true) .setHeaderCommentStatements( - ServiceStubSettingsCommentComposer.createBuilderClassComment( - getThisClassName(service.name()))) + SettingsCommentComposer.createBuilderClassComment(getThisClassName(service.name()))) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setName(className) @@ -1665,7 +1657,7 @@ private static MethodDefinition createNestedClassApplyToAllUnaryMethodsMethod( return MethodDefinition.builder() .setHeaderCommentStatements( - ServiceStubSettingsCommentComposer.APPLY_TO_ALL_UNARY_METHODS_METHOD_COMMENTS) + SettingsCommentComposer.APPLY_TO_ALL_UNARY_METHODS_METHOD_COMMENTS) .setScope(ScopeNode.PUBLIC) .setReturnType(returnType) .setName(methodName) @@ -1713,7 +1705,7 @@ private static List createNestedClassSettingsBuilderGetterMeth javaMethods.add( MethodDefinition.builder() .setHeaderCommentStatements( - ServiceStubSettingsCommentComposer.createCallSettingsBuilderGetterComment( + SettingsCommentComposer.createCallSettingsBuilderGetterComment( getMethodNameFromSettingsVarName(varName))) .setAnnotations(isOperationCallSettings ? lroBetaAnnotations : ImmutableList.of()) .setScope(ScopeNode.PUBLIC) diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java index 6490109316..110d54f7d5 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposerTest.java @@ -87,45 +87,75 @@ public void generateServiceClasses() { + "import java.util.List;\n" + "import javax.annotation.Generated;\n" + "\n" + + "// AUTO-GENERATED DOCUMENTATION AND CLASS.\n" + + "/**\n" + + " * Settings class to configure an instance of {@link EchoClient}.\n" + + " *\n" + + " *

The default instance has everything set to sensible defaults:\n" + + " *\n" + + " *

    \n" + + " *
  • The default service address (localhost) and default port (7469) are used.\n" + + " *
  • Credentials are acquired automatically through Application Default" + + " Credentials.\n" + + " *
  • Retries are configured for idempotent methods but not for non-idempotent" + + " methods.\n" + + " *
\n" + + " *\n" + + " *

The builder of this class is recursive, so contained classes are themselves" + + " builders. When\n" + + " * build() is called, the tree of builders is called to create the complete settings" + + " object.\n" + + " *\n" + + " *

For example, to set the total timeout of echo to 30 seconds:\n" + + " */\n" + "@Generated(\"by gapic-generator-java\")\n" + "public class EchoSettings extends ClientSettings {\n" + "\n" + + " /** Returns the object with the settings used for calls to echo. */\n" + " public UnaryCallSettings echoSettings() {\n" + " return ((EchoStubSettings) getStubSettings()).echoSettings();\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to expand. */\n" + " public ServerStreamingCallSettings expandSettings()" + " {\n" + " return ((EchoStubSettings) getStubSettings()).expandSettings();\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to collect. */\n" + " public StreamingCallSettings collectSettings() {\n" + " return ((EchoStubSettings) getStubSettings()).collectSettings();\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to chat. */\n" + " public StreamingCallSettings chatSettings() {\n" + " return ((EchoStubSettings) getStubSettings()).chatSettings();\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to chatAgain. */\n" + " public StreamingCallSettings chatAgainSettings() {\n" + " return ((EchoStubSettings) getStubSettings()).chatAgainSettings();\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to pagedExpand. */\n" + " public PagedCallSettings\n" + " pagedExpandSettings() {\n" + " return ((EchoStubSettings) getStubSettings()).pagedExpandSettings();\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to wait. */\n" + " public UnaryCallSettings waitSettings() {\n" + " return ((EchoStubSettings) getStubSettings()).waitSettings();\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to wait. */\n" + " public OperationCallSettings" + " waitOperationSettings() {\n" + " return ((EchoStubSettings) getStubSettings()).waitOperationSettings();\n" + " }\n" + "\n" + + " /** Returns the object with the settings used for calls to block. */\n" + " public UnaryCallSettings blockSettings() {\n" + " return ((EchoStubSettings) getStubSettings()).blockSettings();\n" + " }\n" @@ -135,24 +165,29 @@ public void generateServiceClasses() { + " return new EchoStubSettings.Builder(stub.toBuilder()).build();\n" + " }\n" + "\n" + + " /** Returns a builder for the default ExecutorProvider for this service. */\n" + " public static InstantiatingExecutorProvider.Builder" + " defaultExecutorProviderBuilder() {\n" + " return EchoStubSettings.defaultExecutorProviderBuilder();\n" + " }\n" + "\n" + + " /** Returns the default service endpoint. */\n" + " public static String getDefaultEndpoint() {\n" + " return EchoStubSettings.getDefaultEndpoint();\n" + " }\n" + "\n" + + " /** Returns the default service scopes. */\n" + " public static List getDefaultServiceScopes() {\n" + " return EchoStubSettings.getDefaultServiceScopes();\n" + " }\n" + "\n" + + " /** Returns a builder for the default credentials for this service. */\n" + " public static GoogleCredentialsProvider.Builder defaultCredentialsProviderBuilder()" + " {\n" + " return EchoStubSettings.defaultCredentialsProviderBuilder();\n" + " }\n" + "\n" + + " /** Returns a builder for the default ChannelProvider for this service. */\n" + " public static InstantiatingGrpcChannelProvider.Builder" + " defaultGrpcTransportProviderBuilder() {\n" + " return EchoStubSettings.defaultGrpcTransportProviderBuilder();\n" @@ -169,14 +204,17 @@ public void generateServiceClasses() { + " return EchoStubSettings.defaultApiClientHeaderProviderBuilder();\n" + " }\n" + "\n" + + " /** Returns a new builder for this class. */\n" + " public static Builder newBuilder() {\n" + " return Builder.createDefault();\n" + " }\n" + "\n" + + " /** Returns a new builder for this class. */\n" + " public static Builder newBuilder(ClientContext clientContext) {\n" + " return new Builder(clientContext);\n" + " }\n" + "\n" + + " /** Returns a builder containing all the values of this settings class. */\n" + " public Builder toBuilder() {\n" + " return new Builder(this);\n" + " }\n" @@ -185,6 +223,7 @@ public void generateServiceClasses() { + " super(settingsBuilder);\n" + " }\n" + "\n" + + " /** Builder for EchoSettings. */\n" + " public static class Builder extends ClientSettings.Builder" + " {\n" + "\n" @@ -212,6 +251,14 @@ public void generateServiceClasses() { + " return ((EchoStubSettings.Builder) getStubSettings());\n" + " }\n" + "\n" + + " // NEXT_MAJOR_VER: remove 'throws Exception'.\n" + + " /**\n" + + " * Applies the given settings updater function to all of the unary API methods in" + + " this service.\n" + + " *\n" + + " *

Note: This method does not support applying settings to streaming" + + " methods.\n" + + " */\n" + " public Builder applyToAllUnaryMethods(\n" + " ApiFunction, Void> settingsUpdater) throws" + " Exception {\n" @@ -220,45 +267,54 @@ public void generateServiceClasses() { + " return this;\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to echo. */\n" + " public UnaryCallSettings.Builder echoSettings() {\n" + " return getStubSettingsBuilder().echoSettings();\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to expand. */\n" + " public ServerStreamingCallSettings.Builder" + " expandSettings() {\n" + " return getStubSettingsBuilder().expandSettings();\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to collect. */\n" + " public StreamingCallSettings.Builder collectSettings()" + " {\n" + " return getStubSettingsBuilder().collectSettings();\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to chat. */\n" + " public StreamingCallSettings.Builder chatSettings()" + " {\n" + " return getStubSettingsBuilder().chatSettings();\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to chatAgain. */\n" + " public StreamingCallSettings.Builder" + " chatAgainSettings() {\n" + " return getStubSettingsBuilder().chatAgainSettings();\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to pagedExpand. */\n" + " public PagedCallSettings.Builder<\n" + " PagedExpandRequest, PagedExpandResponse, PagedExpandPagedResponse>\n" + " pagedExpandSettings() {\n" + " return getStubSettingsBuilder().pagedExpandSettings();\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to wait. */\n" + " public UnaryCallSettings.Builder waitSettings() {\n" + " return getStubSettingsBuilder().waitSettings();\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to wait. */\n" + " public OperationCallSettings.Builder\n" + " waitOperationSettings() {\n" + " return getStubSettingsBuilder().waitOperationSettings();\n" + " }\n" + "\n" + + " /** Returns the builder for the settings used for calls to block. */\n" + " public UnaryCallSettings.Builder blockSettings() {\n" + " return getStubSettingsBuilder().blockSettings();\n" + " }\n" From 5bdad77d4603ec02a99a0ac86a614bc1d359d5f5 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 3 Sep 2020 15:29:42 -0700 Subject: [PATCH 14/15] fix: file move --- ...SerttingsCommentComposer.java => SettingsCommentComposer.java} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/main/java/com/google/api/generator/gapic/composer/{SerttingsCommentComposer.java => SettingsCommentComposer.java} (100%) diff --git a/src/main/java/com/google/api/generator/gapic/composer/SerttingsCommentComposer.java b/src/main/java/com/google/api/generator/gapic/composer/SettingsCommentComposer.java similarity index 100% rename from src/main/java/com/google/api/generator/gapic/composer/SerttingsCommentComposer.java rename to src/main/java/com/google/api/generator/gapic/composer/SettingsCommentComposer.java From 2ea07c87bdeda0247f803fbe7b1cc092ab7c44e7 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 3 Sep 2020 15:44:45 -0700 Subject: [PATCH 15/15] feat: add comments for GrpcServiceCallableFactory, GrpcServiceStub, ServiceStub --- ...pcServiceCallableFactoryClassComposer.java | 3 + .../GrpcServiceStubClassComposer.java | 2 + .../composer/ServiceStubClassComposer.java | 2 + .../gapic/composer/StubComentComposer.java | 65 +++++++++++++++++++ ...rviceCallableFactoryClassComposerTest.java | 6 ++ .../GrpcServiceStubClassComposerTest.java | 6 ++ .../ServiceStubClassComposerTest.java | 6 ++ 7 files changed, 90 insertions(+) create mode 100644 src/main/java/com/google/api/generator/gapic/composer/StubComentComposer.java diff --git a/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceCallableFactoryClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceCallableFactoryClassComposer.java index 8263b17312..5d1f054e2f 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceCallableFactoryClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceCallableFactoryClassComposer.java @@ -73,6 +73,9 @@ public GapicClass generate(Service service, Map ignore) { ClassDefinition classDef = ClassDefinition.builder() .setPackageString(pakkage) + .setHeaderCommentStatements( + StubCommentComposer.createGrpcServiceCallableFactoryClassHeaderComments( + service.name())) .setAnnotations(createClassAnnotations(types)) .setImplementsTypes(createClassImplements(types)) .setName(className) diff --git a/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java index 5d47bbe9b3..8b7eb4f31e 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java @@ -140,6 +140,8 @@ public GapicClass generate(Service service, Map ignore) { ClassDefinition classDef = ClassDefinition.builder() .setPackageString(pakkage) + .setHeaderCommentStatements( + StubCommentComposer.createGrpcServiceStubClassHeaderComments(service.name())) .setAnnotations(createClassAnnotations()) .setScope(ScopeNode.PUBLIC) .setName(className) diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubClassComposer.java index f2998431ae..cf849c947b 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubClassComposer.java @@ -65,6 +65,8 @@ public GapicClass generate(Service service, Map messageTypes) { ClassDefinition classDef = ClassDefinition.builder() .setPackageString(pakkage) + .setHeaderCommentStatements( + StubCommentComposer.createServiceStubClassHeaderComments(service.name())) .setAnnotations(createClassAnnotations(types)) .setIsAbstract(true) .setImplementsTypes(createClassImplements(types)) diff --git a/src/main/java/com/google/api/generator/gapic/composer/StubComentComposer.java b/src/main/java/com/google/api/generator/gapic/composer/StubComentComposer.java new file mode 100644 index 0000000000..fb7731496d --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/composer/StubComentComposer.java @@ -0,0 +1,65 @@ +// 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.generator.engine.ast.CommentStatement; +import com.google.api.generator.engine.ast.JavaDocComment; +import java.util.Arrays; +import java.util.List; + +class StubCommentComposer { + private static final String STUB_CLASS_HEADER_SUMMARY_PATTERN = + "Base stub class for the %s service API."; + private static final String GRPC_CALLABLE_FACTORY_CLASS_HEADER_SUMMARY_PATTERN = + "gRPC callable factory implementation for the %s service API."; + private static final String GRPC_STUB_CLASS_HEADER_SUMMARY_PATTERN = + "gRPC stub implementation for the %s service API."; + + private static final String ADVANCED_USAGE_DESCRIPTION = "This class is for advanced usage."; + private static final String ADVANCED_USAGE_API_REFLECTION_DESCRIPTION = + "This class is for advanced usage and reflects the underlying API directly."; + + static List createGrpcServiceStubClassHeaderComments(String serviceName) { + return Arrays.asList( + CommentComposer.AUTO_GENERATED_CLASS_COMMENT, + CommentStatement.withComment( + JavaDocComment.builder() + .addComment(String.format(GRPC_STUB_CLASS_HEADER_SUMMARY_PATTERN, serviceName)) + .addParagraph(ADVANCED_USAGE_API_REFLECTION_DESCRIPTION) + .build())); + } + + static List createGrpcServiceCallableFactoryClassHeaderComments( + String serviceName) { + return Arrays.asList( + CommentComposer.AUTO_GENERATED_CLASS_COMMENT, + CommentStatement.withComment( + JavaDocComment.builder() + .addComment( + String.format(GRPC_CALLABLE_FACTORY_CLASS_HEADER_SUMMARY_PATTERN, serviceName)) + .addParagraph(ADVANCED_USAGE_DESCRIPTION) + .build())); + } + + static List createServiceStubClassHeaderComments(String serviceName) { + return Arrays.asList( + CommentComposer.AUTO_GENERATED_CLASS_COMMENT, + CommentStatement.withComment( + JavaDocComment.builder() + .addComment(String.format(STUB_CLASS_HEADER_SUMMARY_PATTERN, serviceName)) + .addParagraph(ADVANCED_USAGE_API_REFLECTION_DESCRIPTION) + .build())); + } +} diff --git a/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceCallableFactoryClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceCallableFactoryClassComposerTest.java index c935ceca41..33bb12caad 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceCallableFactoryClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceCallableFactoryClassComposerTest.java @@ -83,6 +83,12 @@ public void generateServiceClasses() { + "import com.google.longrunning.stub.OperationsStub;\n" + "import javax.annotation.Generated;\n" + "\n" + + "// AUTO-GENERATED DOCUMENTATION AND CLASS.\n" + + "/**\n" + + " * gRPC callable factory implementation for the Echo service API.\n" + + " *\n" + + " *

This class is for advanced usage.\n" + + " */\n" + "@Generated(\"by gapic-generator\")\n" + "public class GrpcEchoCallableFactory implements GrpcStubCallableFactory {\n" + "\n" diff --git a/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposerTest.java index 81b04fbba5..9bd357bc15 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposerTest.java @@ -93,6 +93,12 @@ public void generateServiceClasses() { + "import java.util.concurrent.TimeUnit;\n" + "import javax.annotation.Generated;\n" + "\n" + + "// AUTO-GENERATED DOCUMENTATION AND CLASS.\n" + + "/**\n" + + " * gRPC stub implementation for the Echo service API.\n" + + " *\n" + + " *

This class is for advanced usage and reflects the underlying API directly.\n" + + " */\n" + "@Generated(\"by gapic-generator-java\")\n" + "public class GrpcEchoStub extends EchoStub {\n" + " private static final MethodDescriptor" diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubClassComposerTest.java index 54d9ef5051..8ce0fac92c 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubClassComposerTest.java @@ -82,6 +82,12 @@ public void generateServiceClasses() { + "import com.google.showcase.v1beta1.WaitResponse;\n" + "import javax.annotation.Generated;\n" + "\n" + + "// AUTO-GENERATED DOCUMENTATION AND CLASS.\n" + + "/**\n" + + " * Base stub class for the Echo service API.\n" + + " *\n" + + " *

This class is for advanced usage and reflects the underlying API directly.\n" + + " */\n" + "@Generated(\"by gapic-generator\")\n" + "public abstract class EchoStub implements BackgroundResource {\n" + "\n"