From c71f061f96d8b418c8c1549cf65eb9fc45ee6396 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 25 Aug 2020 13:06:47 -0700 Subject: [PATCH 01/24] feat: add factory var decl in ServiceStubSettings codegen --- .../ServiceStubSettingsClassComposer.java | 223 +++++++++++++++++- .../ServiceStubSettingsClassComposerTest.java | 26 ++ 2 files changed, 242 insertions(+), 7 deletions(-) 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 9762def49a..79ea6a0117 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 @@ -52,6 +52,7 @@ import com.google.api.gax.rpc.UnaryCallSettings; import com.google.api.gax.rpc.UnaryCallable; import com.google.api.generator.engine.ast.AnnotationNode; +import com.google.api.generator.engine.ast.AnonymousClassExpr; import com.google.api.generator.engine.ast.AssignmentExpr; import com.google.api.generator.engine.ast.ClassDefinition; import com.google.api.generator.engine.ast.ConcreteReference; @@ -74,6 +75,7 @@ import com.google.api.generator.engine.ast.VaporReference; 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.GapicClass; import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.Method; @@ -102,9 +104,11 @@ // TODO(miraleung): Refactor ClassComposer's interface. public class ServiceStubSettingsClassComposer { private static final String CLASS_NAME_PATTERN = "%sStubSettings"; + private static final String GRPC_SERVICE_STUB_PATTERN = "Grpc%sStub"; + private static final String PAGE_STR_DESC_PATTERN = "%s_PAGE_STR_DESC"; + private static final String PAGED_RESPONSE_FACTORY_PATTERN = "%s_PAGE_STR_FACT"; private static final String PAGED_RESPONSE_TYPE_NAME_PATTERN = "%sPagedResponse"; private static final String NESTED_BUILDER_CLASS_NAME = "Builder"; - private static final String GRPC_SERVICE_STUB_PATTERN = "Grpc%sStub"; private static final String STUB_PATTERN = "%sStub"; private static final String LEFT_BRACE = "{"; @@ -139,7 +143,9 @@ public GapicClass generate( .setScope(ScopeNode.PUBLIC) .setName(className) .setExtendsType(createExtendsType(service, types)) - .setStatements(createClassStatements(service, methodSettingsMemberVarExprs, types)) + .setStatements( + createClassStatements( + service, serviceConfig, methodSettingsMemberVarExprs, messageTypes, types)) .setMethods(createClassMethods(service, methodSettingsMemberVarExprs, types)) .setNestedClasses(Arrays.asList(createNestedBuilderClass(service, types))) .build(); @@ -193,9 +199,11 @@ private static Map createClassMemberVarExprs( private static List createClassStatements( Service service, + ServiceConfig serviceConfig, Map methodSettingsMemberVarExprs, + Map messageTypes, Map types) { - List memberVars = new ArrayList<>(); + List memberVarExprs = new ArrayList<>(); // Assign DEFAULT_SERVICE_SCOPES. VariableExpr defaultServiceScopesDeclVarExpr = @@ -228,14 +236,14 @@ private static List createClassStatements( .setReturnType(DEFAULT_SERVICE_SCOPES_VAR_EXPR.type()) .build(); - memberVars.add( + memberVarExprs.add( AssignmentExpr.builder() .setVariableExpr(defaultServiceScopesDeclVarExpr) .setValueExpr(listBuilderExpr) .build()); // Declare settings members. - memberVars.addAll( + memberVarExprs.addAll( methodSettingsMemberVarExprs.values().stream() .map( v -> @@ -246,8 +254,209 @@ private static List createClassStatements( .build()) .collect(Collectors.toList())); - // TODO(miraleung): Fill this out. - return memberVars.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList()); + memberVarExprs.addAll( + createPagingStaticAssignExprs(service, serviceConfig, messageTypes, types)); + return memberVarExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList()); + } + + private static List createPagingStaticAssignExprs( + Service service, + ServiceConfig serviceConfig, + Map messageTypes, + Map types) { + // TODO(miraleung): Add a test case for several such statements. + List exprs = new ArrayList<>(); + for (Method method : service.methods()) { + if (!method.isPaged()) { + continue; + } + // Find the repeated type. + Message pagedResponseMessage = + messageTypes.get(JavaStyle.toUpperCamelCase(method.outputType().reference().name())); + TypeNode repeatedResponseType = null; + for (Field field : pagedResponseMessage.fields()) { + if (field.isRepeated()) { + // Field is currently a List-type. + Preconditions.checkState( + !field.type().reference().generics().isEmpty(), + String.format("No generics found for field reference %s", field.type().reference())); + repeatedResponseType = TypeNode.withReference(field.type().reference().generics().get(0)); + } + } + Preconditions.checkNotNull( + repeatedResponseType, + String.format( + "No repeated type found for paged reesponse %s for method %s", + method.outputType().reference().name(), method.name())); + + // Create the PAGE_STR_DESC variable. + TypeNode pagedListDescriptorType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(PagedListDescriptor.class) + .setGenerics( + Arrays.asList(method.inputType(), method.outputType(), repeatedResponseType) + .stream() + .map(t -> t.reference()) + .collect(Collectors.toList())) + .build()); + String pageStrDescVarName = + String.format(PAGE_STR_DESC_PATTERN, JavaStyle.toUpperSnakeCase(method.name())); + VariableExpr pageStrDescVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setType(pagedListDescriptorType) + .setName(pageStrDescVarName) + .build()); + + Expr pagedListResponseFactoryAssignExpr = + createPagedListResponseFactoryAssignExpr( + pageStrDescVarExpr, method, repeatedResponseType, types); + + exprs.add(pagedListResponseFactoryAssignExpr); + } + + return exprs; + } + + private static Expr createPagedListResponseFactoryAssignExpr( + VariableExpr pageStrDescVarExpr, + Method method, + TypeNode repeatedResponseType, + Map types) { + Preconditions.checkState( + method.isPaged(), String.format("Method %s is not paged", method.name())); + + // Create the PagedListResponseFactory. + TypeNode pagedResponseType = types.get(getPagedResponseTypeName(method.name())); + TypeNode apiFutureType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(ApiFuture.class) + .setGenerics(Arrays.asList(pagedResponseType.reference())) + .build()); + + VariableExpr callableVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setType( + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(UnaryCallable.class) + .setGenerics( + Arrays.asList( + method.inputType().reference(), + method.outputType().reference())) + .build())) + .setName("callable") + .build()); + VariableExpr requestVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(method.inputType()).setName("request").build()); + VariableExpr contextVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setType(STATIC_TYPES.get("ApiCallContext")) + .setName("context") + .build()); + VariableExpr futureResponseVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setType( + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(ApiFuture.class) + .setGenerics(Arrays.asList(method.outputType().reference())) + .build())) + .setName("futureResponse") + .build()); + + TypeNode pageContextType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(PageContext.class) + .setGenerics( + Arrays.asList(method.inputType(), method.outputType(), repeatedResponseType) + .stream() + .map(t -> t.reference()) + .collect(Collectors.toList())) + .build()); + VariableExpr pageContextVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(pageContextType).setName("pageContext").build()); + AssignmentExpr pageContextAssignExpr = + AssignmentExpr.builder() + .setVariableExpr(pageContextVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr( + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("PageContext")) + .setMethodName("create") + .setArguments( + callableVarExpr, pageStrDescVarExpr, requestVarExpr, contextVarExpr) + .setReturnType(pageContextVarExpr.type()) + .build()) + .build(); + + Expr returnExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(types.get(getPagedResponseTypeName(method.name()))) + .setMethodName("createAsync") + .setArguments(pageContextVarExpr, futureResponseVarExpr) + .setReturnType(apiFutureType) + .build(); + + MethodDefinition getFuturePagedResponseMethod = + MethodDefinition.builder() + .setIsOverride(true) + .setScope(ScopeNode.PUBLIC) + .setReturnType(apiFutureType) + .setName("getFuturePagedResponse") + .setArguments( + Arrays.asList( + callableVarExpr, requestVarExpr, contextVarExpr, futureResponseVarExpr) + .stream() + .map(v -> v.toBuilder().setIsDecl(true).build()) + .collect(Collectors.toList())) + .setBody(Arrays.asList(ExprStatement.withExpr(pageContextAssignExpr))) + .setReturnExpr(returnExpr) + .build(); + + // Create the variable. + TypeNode pagedResponseFactoryType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(PagedListResponseFactory.class) + .setGenerics( + Arrays.asList( + method.inputType(), + method.outputType(), + types.get(getPagedResponseTypeName(method.name()))) + .stream() + .map(t -> t.reference()) + .collect(Collectors.toList())) + .build()); + String varName = + String.format(PAGED_RESPONSE_FACTORY_PATTERN, JavaStyle.toUpperSnakeCase(method.name())); + VariableExpr pagedListResponseFactoryVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(pagedResponseFactoryType).setName(varName).build()); + AnonymousClassExpr factoryAnonClassExpr = + AnonymousClassExpr.builder() + .setType(pagedResponseFactoryType) + .setMethods(Arrays.asList(getFuturePagedResponseMethod)) + .build(); + + return AssignmentExpr.builder() + .setVariableExpr( + pagedListResponseFactoryVarExpr + .toBuilder() + .setIsDecl(true) + .setScope(ScopeNode.PRIVATE) + .setIsStatic(true) + .setIsFinal(true) + .build()) + .setValueExpr(factoryAnonClassExpr) + .build(); } private static List createClassMethods( 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 6759638b23..7c4738074f 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 @@ -82,6 +82,7 @@ public void generateServiceClasses() { + "\n" + "import static com.google.showcase.v1beta1.EchoClient.PagedExpandPagedResponse;\n" + "\n" + + "import com.google.api.core.ApiFuture;\n" + "import com.google.api.core.BetaApi;\n" + "import com.google.api.gax.core.GaxProperties;\n" + "import com.google.api.gax.core.GoogleCredentialsProvider;\n" @@ -89,15 +90,20 @@ public void generateServiceClasses() { + "import com.google.api.gax.grpc.GaxGrpcProperties;\n" + "import com.google.api.gax.grpc.GrpcTransportChannel;\n" + "import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;\n" + + "import com.google.api.gax.rpc.ApiCallContext;\n" + "import com.google.api.gax.rpc.ApiClientHeaderProvider;\n" + "import com.google.api.gax.rpc.ClientContext;\n" + "import com.google.api.gax.rpc.OperationCallSettings;\n" + + "import com.google.api.gax.rpc.PageContext;\n" + "import com.google.api.gax.rpc.PagedCallSettings;\n" + + "import com.google.api.gax.rpc.PagedListDescriptor;\n" + + "import com.google.api.gax.rpc.PagedListResponseFactory;\n" + "import com.google.api.gax.rpc.ServerStreamingCallSettings;\n" + "import com.google.api.gax.rpc.StreamingCallSettings;\n" + "import com.google.api.gax.rpc.StubSettings;\n" + "import com.google.api.gax.rpc.TransportChannelProvider;\n" + "import com.google.api.gax.rpc.UnaryCallSettings;\n" + + "import com.google.api.gax.rpc.UnaryCallable;\n" + "import com.google.common.collect.ImmutableList;\n" + "import com.google.longrunning.Operation;\n" + "import com.google.showcase.v1beta1.BlockRequest;\n" @@ -133,6 +139,26 @@ public void generateServiceClasses() { + " private final OperationCallSettings\n" + " waitOperationSettings;\n" + " private final UnaryCallSettings blockSettings;\n" + + " private static final PagedListResponseFactory<\n" + + " PagedExpandRequest, PagedExpandResponse, PagedExpandPagedResponse>\n" + + " PAGED_EXPAND_PAGE_STR_FACT =\n" + + " new PagedListResponseFactory<\n" + + " PagedExpandRequest, PagedExpandResponse, PagedExpandPagedResponse>()" + + " {\n" + + " @Override\n" + + " public ApiFuture getFuturePagedResponse(\n" + + " UnaryCallable callable,\n" + + " PagedExpandRequest request,\n" + + " ApiCallContext context,\n" + + " ApiFuture futureResponse) {\n" + + " PageContext" + + " pageContext =\n" + + " PageContext.create(callable, PAGED_EXPAND_PAGE_STR_DESC, request," + + " context);\n" + + " return PagedExpandPagedResponse.createAsync(pageContext," + + " futureResponse);\n" + + " }\n" + + " };\n" + "\n" + " public UnaryCallSettings echoSettings() {\n" + " return echoSettings;\n" From 4919bba7b3f59327f4338383a2265ac84f977e25 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 25 Aug 2020 15:17:07 -0700 Subject: [PATCH 02/24] fix: prevent duplicate MethodDefinition annotations --- .../engine/ast/MethodDefinition.java | 22 +++++++++++++------ .../engine/ast/MethodDefinitionTest.java | 22 ++++++++++++++++++- 2 files changed, 36 insertions(+), 8 deletions(-) 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 c37ab6579f..285fc898dd 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 @@ -19,6 +19,7 @@ import com.google.common.collect.ImmutableList; import java.util.Arrays; import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; import java.util.stream.Collectors; import javax.annotation.Nullable; @@ -122,10 +123,7 @@ public abstract static class Builder { public abstract Builder setHeaderCommentStatements( List headeCommentStatements); - public Builder setAnnotations(List annotations) { - annotationsBuilder().addAll(annotations); - return this; - } + public abstract Builder setAnnotations(List annotations); public abstract Builder setIsStatic(boolean isStatic); @@ -162,10 +160,10 @@ public Builder setArguments(VariableExpr... arguments) { // Private accessors. - abstract ImmutableList.Builder annotationsBuilder(); - abstract String name(); + abstract ImmutableList annotations(); + abstract TypeNode returnType(); abstract boolean isOverride(); @@ -231,9 +229,19 @@ public MethodDefinition build() { } // If this method overrides another, ensure that the Override annotaiton is the last one. + ImmutableList processedAnnotations = annotations(); if (isOverride()) { - annotationsBuilder().add(AnnotationNode.OVERRIDE); + processedAnnotations = + annotations() + .builder() + .addAll(annotations()) + .add(AnnotationNode.OVERRIDE) + .build(); } + // Remove duplicates while maintaining insertion order. + setAnnotations( + new LinkedHashSet(processedAnnotations) + .stream().collect(Collectors.toList())); MethodDefinition method = autoBuild(); diff --git a/src/test/java/com/google/api/generator/engine/ast/MethodDefinitionTest.java b/src/test/java/com/google/api/generator/engine/ast/MethodDefinitionTest.java index ddc92a1eee..4fe7a5c05b 100644 --- a/src/test/java/com/google/api/generator/engine/ast/MethodDefinitionTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/MethodDefinitionTest.java @@ -14,6 +14,7 @@ package com.google.api.generator.engine.ast; +import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; import java.util.ArrayList; @@ -35,6 +36,24 @@ public void validMethodDefinition_basicWithComments() { // No exception thrown, we're good. } + @Test + public void validMethodDefinition_repeatedAnnotations() { + MethodDefinition method = + MethodDefinition.builder() + .setName("close") + .setAnnotations( + Arrays.asList( + AnnotationNode.withSuppressWarnings("all"), + AnnotationNode.DEPRECATED, + AnnotationNode.DEPRECATED)) + .setScope(ScopeNode.PUBLIC) + .setReturnType(TypeNode.VOID) + .setBody(Arrays.asList(ExprStatement.withExpr(createAssignmentExpr()))) + .build(); + assertThat(method.annotations()) + .containsExactly(AnnotationNode.withSuppressWarnings("all"), AnnotationNode.DEPRECATED); + } + @Test public void validMethodDefinition_basicWithReturnType() { MethodDefinition.builder() @@ -681,7 +700,8 @@ private static List createCommentStatements() { JavaDocComment.builder() .addComment("Constructs an instance of GrpcMyProtoStub, using the given settings.") .addComment( - "This is protected so that it is easy to make a subclass, but otherwise, the static factory methods should be preferred.") + "This is protected so that it is easy to make a subclass, but otherwise, the" + + " static factory methods should be preferred.") .build(); return Arrays.asList(CommentStatement.withComment(javaDocComment)); } From b77848e68a3815e274b4ba90a7a7f34636166cba Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 25 Aug 2020 15:22:47 -0700 Subject: [PATCH 03/24] feat: add descriptor fields to ServiceStubSettings codegen --- .../ServiceStubSettingsClassComposer.java | 209 +++++++++++++++++- .../ServiceStubSettingsClassComposerTest.java | 43 ++++ 2 files changed, 240 insertions(+), 12 deletions(-) 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 79ea6a0117..6a319ca649 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 @@ -62,12 +62,14 @@ 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.NullObjectValue; import com.google.api.generator.engine.ast.Reference; import com.google.api.generator.engine.ast.ReferenceConstructorExpr; import com.google.api.generator.engine.ast.ReturnExpr; import com.google.api.generator.engine.ast.ScopeNode; import com.google.api.generator.engine.ast.Statement; import com.google.api.generator.engine.ast.StringObjectValue; +import com.google.api.generator.engine.ast.TernaryExpr; import com.google.api.generator.engine.ast.ThisObjectValue; import com.google.api.generator.engine.ast.ThrowExpr; import com.google.api.generator.engine.ast.TypeNode; @@ -96,6 +98,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.function.Function; import java.util.stream.Collectors; import javax.annotation.Generated; @@ -265,7 +268,8 @@ private static List createPagingStaticAssignExprs( Map messageTypes, Map types) { // TODO(miraleung): Add a test case for several such statements. - List exprs = new ArrayList<>(); + List descExprs = new ArrayList<>(); + List factoryExprs = new ArrayList<>(); for (Method method : service.methods()) { if (!method.isPaged()) { continue; @@ -290,7 +294,7 @@ private static List createPagingStaticAssignExprs( method.outputType().reference().name(), method.name())); // Create the PAGE_STR_DESC variable. - TypeNode pagedListDescriptorType = + TypeNode pagedListDescType = TypeNode.withReference( ConcreteReference.builder() .setClazz(PagedListDescriptor.class) @@ -302,21 +306,202 @@ private static List createPagingStaticAssignExprs( .build()); String pageStrDescVarName = String.format(PAGE_STR_DESC_PATTERN, JavaStyle.toUpperSnakeCase(method.name())); - VariableExpr pageStrDescVarExpr = + VariableExpr pagedListDescVarExpr = VariableExpr.withVariable( - Variable.builder() - .setType(pagedListDescriptorType) - .setName(pageStrDescVarName) - .build()); + Variable.builder().setType(pagedListDescType).setName(pageStrDescVarName).build()); - Expr pagedListResponseFactoryAssignExpr = + descExprs.add( + createPagedListDescriptorAssignExpr( + pagedListDescVarExpr, method, repeatedResponseType, types)); + factoryExprs.add( createPagedListResponseFactoryAssignExpr( - pageStrDescVarExpr, method, repeatedResponseType, types); - - exprs.add(pagedListResponseFactoryAssignExpr); + pagedListDescVarExpr, method, repeatedResponseType, types)); } - return exprs; + descExprs.addAll(factoryExprs); + return descExprs; + } + + private static Expr createPagedListDescriptorAssignExpr( + VariableExpr pagedListDescVarExpr, + Method method, + TypeNode repeatedResponseType, + Map types) { + MethodDefinition.Builder methodStarterBuilder = + MethodDefinition.builder().setIsOverride(true).setScope(ScopeNode.PUBLIC); + List anonClassMethods = new ArrayList<>(); + + // Create emptyToken method. + anonClassMethods.add( + methodStarterBuilder + .setReturnType(TypeNode.STRING) + .setName("emptyToken") + .setReturnExpr(ValueExpr.withValue(StringObjectValue.withValue(""))) + .build()); + + // Create injectToken method. + VariableExpr payloadVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(method.inputType()).setName("payload").build()); + VariableExpr strTokenVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(TypeNode.STRING).setName("token").build()); + TypeNode returnType = method.inputType(); + Expr newBuilderExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(method.inputType()) + .setMethodName("newBuilder") + .setArguments(payloadVarExpr) + .build(); + Expr returnExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(newBuilderExpr) + .setMethodName("setPageToken") + .setArguments(strTokenVarExpr) + .build(); + returnExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(returnExpr) + .setMethodName("build") + .setReturnType(returnType) + .build(); + anonClassMethods.add( + methodStarterBuilder + .setReturnType(method.inputType()) + .setName("injectToken") + .setArguments( + Arrays.asList(payloadVarExpr, strTokenVarExpr).stream() + .map(v -> v.toBuilder().setIsDecl(true).build()) + .collect(Collectors.toList())) + .setReturnExpr(returnExpr) + .build()); + + // Create injectPageSize method. + VariableExpr pageSizeVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(TypeNode.INT).setName("pageSize").build()); + // Re-declare for clarity and easier readeability. + returnType = method.inputType(); + returnExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(newBuilderExpr) + .setMethodName("setPageSize") + .setArguments(pageSizeVarExpr) + .build(); + returnExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(returnExpr) + .setMethodName("build") + .setReturnType(returnType) + .build(); + anonClassMethods.add( + methodStarterBuilder + .setReturnType(method.inputType()) + .setName("injectPageSize") + .setArguments( + Arrays.asList(payloadVarExpr, pageSizeVarExpr).stream() + .map(v -> v.toBuilder().setIsDecl(true).build()) + .collect(Collectors.toList())) + .setReturnExpr(returnExpr) + .build()); + + // TODO(miraleung): Test the edge cases where these proto fields aren't present. + // Create extractPageSize method. + returnType = TypeNode.INT_OBJECT; + anonClassMethods.add( + methodStarterBuilder + .setReturnType(returnType) + .setName("extractPageSize") + .setArguments(payloadVarExpr.toBuilder().setIsDecl(true).build()) + .setReturnExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(payloadVarExpr) + .setMethodName("getPageSize") + .setReturnType(returnType) + .build()) + .build()); + + // Create extractNextToken method. + returnType = TypeNode.STRING; + payloadVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(method.outputType()).setName("payload").build()); + anonClassMethods.add( + methodStarterBuilder + .setReturnType(returnType) + .setName("extractNextToken") + .setArguments(payloadVarExpr.toBuilder().setIsDecl(true).build()) + .setReturnExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(payloadVarExpr) + .setMethodName("getNextPageToken") + .setReturnType(returnType) + .build()) + .build()); + + // Create extractResources method. + returnType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(Iterable.class) + .setGenerics(Arrays.asList(repeatedResponseType.reference())) + .build()); + Expr getResponsesListExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(payloadVarExpr) + .setMethodName("getResponsesList") + .setReturnType(returnType) + .build(); + Expr conditionExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType( + TypeNode.withReference(ConcreteReference.withClazz(Objects.class))) + .setMethodName("equals") + .setArguments(getResponsesListExpr, ValueExpr.withValue(NullObjectValue.create())) + .setReturnType(TypeNode.BOOLEAN) + .build(); + Expr thenExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType( + TypeNode.withReference(ConcreteReference.withClazz(ImmutableList.class))) + .setGenerics(Arrays.asList(repeatedResponseType.reference())) + .setMethodName("of") + .setReturnType(returnType) + .build(); + + returnExpr = + TernaryExpr.builder() + .setConditionExpr(conditionExpr) + .setThenExpr(thenExpr) + .setElseExpr(getResponsesListExpr) + .build(); + anonClassMethods.add( + methodStarterBuilder + .setReturnType(returnType) + .setName("extractResources") + .setArguments(payloadVarExpr.toBuilder().setIsDecl(true).build()) + .setReturnExpr(returnExpr) + .build()); + + // Create the anonymous class. + AnonymousClassExpr pagedListDescAnonClassExpr = + AnonymousClassExpr.builder() + .setType(pagedListDescVarExpr.type()) + .setMethods(anonClassMethods) + .build(); + + // Declare and assign the variable. + return AssignmentExpr.builder() + .setVariableExpr( + pagedListDescVarExpr + .toBuilder() + .setIsDecl(true) + .setScope(ScopeNode.PRIVATE) + .setIsStatic(true) + .setIsFinal(true) + .build()) + .setValueExpr(pagedListDescAnonClassExpr) + .build(); } private static Expr createPagedListResponseFactoryAssignExpr( 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 7c4738074f..d5fa63a595 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 @@ -118,6 +118,7 @@ public void generateServiceClasses() { + "import com.google.showcase.v1beta1.WaitResponse;\n" + "import java.io.IOException;\n" + "import java.util.List;\n" + + "import java.util.Objects;\n" + "import javax.annotation.Generated;\n" + "\n" + "@BetaApi\n" @@ -139,6 +140,48 @@ public void generateServiceClasses() { + " private final OperationCallSettings\n" + " waitOperationSettings;\n" + " private final UnaryCallSettings blockSettings;\n" + + " private static final PagedListDescriptor\n" + + " PAGED_EXPAND_PAGE_STR_DESC =\n" + + " new PagedListDescriptor() {\n" + + " @Override\n" + + " public String emptyToken() {\n" + + " return \"\";\n" + + " }\n" + + "\n" + + " @Override\n" + + " public PagedExpandRequest injectToken(PagedExpandRequest payload, String" + + " token) {\n" + + " return" + + " PagedExpandRequest.newBuilder(payload).setPageToken(token).build();\n" + + " }\n" + + "\n" + + " @Override\n" + + " public PagedExpandRequest injectPageSize(PagedExpandRequest payload, int" + + " pageSize) {\n" + + " return" + + " PagedExpandRequest.newBuilder(payload).setPageSize(pageSize).build();\n" + + " }\n" + + "\n" + + " @Override\n" + + " public Integer extractPageSize(PagedExpandRequest payload) {\n" + + " return payload.getPageSize();\n" + + " }\n" + + "\n" + + " @Override\n" + + " public String extractNextToken(PagedExpandResponse payload) {\n" + + " return payload.getNextPageToken();\n" + + " }\n" + + "\n" + + " @Override\n" + + " public Iterable extractResources(PagedExpandResponse" + + " payload) {\n" + + " return Objects.equals(payload.getResponsesList(), null)\n" + + " ? ImmutableList.of()\n" + + " : payload.getResponsesList();\n" + + " }\n" + + " };\n" + " private static final PagedListResponseFactory<\n" + " PagedExpandRequest, PagedExpandResponse, PagedExpandPagedResponse>\n" + " PAGED_EXPAND_PAGE_STR_FACT =\n" From 861f142f6770c06e47b92f6f0e68b84d8acfe7d2 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 25 Aug 2020 15:59:26 -0700 Subject: [PATCH 04/24] feat: add starter Builder to ServiceStubSettings codegen --- .../ServiceStubSettingsClassComposer.java | 104 ++++++++++++++++++ .../ServiceStubSettingsClassComposerTest.java | 13 ++- 2 files changed, 116 insertions(+), 1 deletion(-) 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 6a319ca649..b153047559 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 @@ -112,6 +112,11 @@ public class ServiceStubSettingsClassComposer { private static final String PAGED_RESPONSE_FACTORY_PATTERN = "%s_PAGE_STR_FACT"; private static final String PAGED_RESPONSE_TYPE_NAME_PATTERN = "%sPagedResponse"; private static final String NESTED_BUILDER_CLASS_NAME = "Builder"; + private static final String NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_NAME = + "unaryMethodSettingsBuilders"; + private static final String NESTED_RETRYABLE_CODE_DEFINITIONS_VAR_NAME = + "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 LEFT_BRACE = "{"; @@ -124,6 +129,12 @@ public class ServiceStubSettingsClassComposer { private static final Map STATIC_TYPES = createStaticTypes(); private static final VariableExpr DEFAULT_SERVICE_SCOPES_VAR_EXPR = createDefaultServiceScopesVarExpr(); + private static final VariableExpr NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR = + createNestedUnaryMethodSettingsBuildersVarExpr(); + private static final VariableExpr NESTED_RETRYABLE_CODE_DEFINITIONS_VAR_EXPR = + createNestedRetryableCodeDefinitionsVarExpr(); + private static final VariableExpr NESTED_RETRY_PARAM_DEFINITIONS_VAR_EXPR = + createNestedRetryParamDefinitionsVarExpr(); private ServiceStubSettingsClassComposer() {} @@ -1048,15 +1059,49 @@ private static ClassDefinition createNestedBuilderClass( String className = "Builder"; + TypeNode extendsType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(StubSettings.Builder.class) + .setGenerics( + Arrays.asList( + types.get(getServiceStubTypeName(service.name())), types.get(className)) + .stream() + .map(t -> t.reference()) + .collect(Collectors.toList())) + .build()); + // TODO(miraleung): Fill this out. return ClassDefinition.builder() .setIsNested(true) .setScope(ScopeNode.PUBLIC) .setIsStatic(true) .setName(className) + .setExtendsType(extendsType) + .setStatements(createNestedClassStatements()) .build(); } + private static List createNestedClassStatements() { + List varDeclExprs = new ArrayList<>(); + Function varDeclFn = + v -> v.toBuilder().setIsDecl(true).setScope(ScopeNode.PRIVATE).setIsFinal(true).build(); + varDeclExprs.add(varDeclFn.apply(NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR)); + + Function varStaticDeclFn = + v -> + v.toBuilder() + .setIsDecl(true) + .setScope(ScopeNode.PRIVATE) + .setIsStatic(true) + .setIsFinal(true) + .build(); + varDeclExprs.add(varStaticDeclFn.apply(NESTED_RETRYABLE_CODE_DEFINITIONS_VAR_EXPR)); + varDeclExprs.add(varStaticDeclFn.apply(NESTED_RETRY_PARAM_DEFINITIONS_VAR_EXPR)); + + return varDeclExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList()); + } + private static Map createStaticTypes() { List concreteClazzes = Arrays.asList( @@ -1179,6 +1224,65 @@ private static VariableExpr createDefaultServiceScopesVarExpr() { Variable.builder().setName("DEFAULT_SERVICE_SCOPES").setType(listStringType).build()); } + private static VariableExpr createNestedUnaryMethodSettingsBuildersVarExpr() { + Reference builderRef = + ConcreteReference.builder() + .setClazz(UnaryCallSettings.Builder.class) + .setGenerics(Arrays.asList(TypeNode.WILDCARD_REFERENCE, TypeNode.WILDCARD_REFERENCE)) + .build(); + TypeNode varType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(ImmutableList.class) + .setGenerics(Arrays.asList(builderRef)) + .build()); + return VariableExpr.withVariable( + Variable.builder() + .setType(varType) + .setName(NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_NAME) + .build()); + } + + private static VariableExpr createNestedRetryableCodeDefinitionsVarExpr() { + TypeNode immutableSetType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(ImmutableSet.class) + .setGenerics(Arrays.asList(ConcreteReference.withClazz(StatusCode.Code.class))) + .build()); + TypeNode varType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(ImmutableMap.class) + .setGenerics( + Arrays.asList(TypeNode.STRING, immutableSetType).stream() + .map(t -> t.reference()) + .collect(Collectors.toList())) + .build()); + return VariableExpr.withVariable( + Variable.builder() + .setType(varType) + .setName(NESTED_RETRYABLE_CODE_DEFINITIONS_VAR_NAME) + .build()); + } + + private static VariableExpr createNestedRetryParamDefinitionsVarExpr() { + TypeNode varType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(ImmutableMap.class) + .setGenerics( + Arrays.asList(TypeNode.STRING, STATIC_TYPES.get("RetrySettings")).stream() + .map(t -> t.reference()) + .collect(Collectors.toList())) + .build()); + return VariableExpr.withVariable( + Variable.builder() + .setType(varType) + .setName(NESTED_RETRY_PARAM_DEFINITIONS_VAR_NAME) + .build()); + } + private static String getThisClassName(String serviceName) { return String.format(CLASS_NAME_PATTERN, JavaStyle.toUpperCamelCase(serviceName)); } 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 d5fa63a595..78222adf5d 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 @@ -90,6 +90,7 @@ public void generateServiceClasses() { + "import com.google.api.gax.grpc.GaxGrpcProperties;\n" + "import com.google.api.gax.grpc.GrpcTransportChannel;\n" + "import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;\n" + + "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.ClientContext;\n" @@ -99,12 +100,15 @@ public void generateServiceClasses() { + "import com.google.api.gax.rpc.PagedListDescriptor;\n" + "import com.google.api.gax.rpc.PagedListResponseFactory;\n" + "import com.google.api.gax.rpc.ServerStreamingCallSettings;\n" + + "import com.google.api.gax.rpc.StatusCode;\n" + "import com.google.api.gax.rpc.StreamingCallSettings;\n" + "import com.google.api.gax.rpc.StubSettings;\n" + "import com.google.api.gax.rpc.TransportChannelProvider;\n" + "import com.google.api.gax.rpc.UnaryCallSettings;\n" + "import com.google.api.gax.rpc.UnaryCallable;\n" + "import com.google.common.collect.ImmutableList;\n" + + "import com.google.common.collect.ImmutableMap;\n" + + "import com.google.common.collect.ImmutableSet;\n" + "import com.google.longrunning.Operation;\n" + "import com.google.showcase.v1beta1.BlockRequest;\n" + "import com.google.showcase.v1beta1.BlockResponse;\n" @@ -323,6 +327,13 @@ public void generateServiceClasses() { + " blockSettings = settingsBuilder.blockSettings().build();\n" + " }\n" + "\n" - + " public static class Builder {}\n" + + " public static class Builder extends StubSettings.Builder {\n" + + " private final ImmutableList>" + + " unaryMethodSettingsBuilders;\n" + + " private static final ImmutableMap>\n" + + " RETRYABLE_CODE_DEFINITIONS;\n" + + " private static final ImmutableMap" + + " RETRY_PARAM_DEFINITIONS;\n" + + " }\n" + "}\n"; } From 5f285df576a33a41076b11e9d5bc2c59cf89fbc9 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 25 Aug 2020 16:18:45 -0700 Subject: [PATCH 05/24] feat: add settings.builder decls to ServiceStubSettings codegen --- .../ServiceStubSettingsClassComposer.java | 62 ++++++++++++++----- .../ServiceStubSettingsClassComposerTest.java | 19 ++++++ 2 files changed, 67 insertions(+), 14 deletions(-) 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 b153047559..311f6fa1dd 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 @@ -147,7 +147,7 @@ public GapicClass generate( String pakkage = String.format("%s.stub", service.pakkage()); Map types = createDynamicTypes(service, pakkage); Map methodSettingsMemberVarExprs = - createClassMemberVarExprs(service, types); + createMethodSettingsClassMemberVarExprs(service, types, /* isNestedClass= */ false); String className = getThisClassName(service.name()); ClassDefinition classDef = @@ -184,22 +184,22 @@ private static TypeNode createExtendsType(Service service, Map .copyAndSetGenerics(Arrays.asList(thisClassType.reference()))); } - private static Map createClassMemberVarExprs( - Service service, Map types) { + private static Map createMethodSettingsClassMemberVarExprs( + Service service, Map types, boolean isNestedClass) { // Maintain insertion order. Map varExprs = new LinkedHashMap<>(); // Creates class variables Settings, e.g. echoSettings. // TODO(miraleung): Handle batching here. for (Method method : service.methods()) { - TypeNode settingsType = getCallSettingsType(method, types); + TypeNode settingsType = getCallSettingsType(method, types, isNestedClass); String varName = JavaStyle.toLowerCamelCase(String.format("%sSettings", method.name())); varExprs.put( varName, VariableExpr.withVariable( Variable.builder().setType(settingsType).setName(varName).build())); if (method.hasLro()) { - settingsType = getOperationCallSettingsType(method); + settingsType = getOperationCallSettingsType(method, isNestedClass); varName = JavaStyle.toLowerCamelCase(String.format("%sOperationSettings", method.name())); varExprs.put( varName, @@ -1071,6 +1071,9 @@ private static ClassDefinition createNestedBuilderClass( .collect(Collectors.toList())) .build()); + Map nestedMethodSettingsMemberVarExprs = + createMethodSettingsClassMemberVarExprs(service, types, /* isNestedClass= */ true); + // TODO(miraleung): Fill this out. return ClassDefinition.builder() .setIsNested(true) @@ -1078,16 +1081,26 @@ private static ClassDefinition createNestedBuilderClass( .setIsStatic(true) .setName(className) .setExtendsType(extendsType) - .setStatements(createNestedClassStatements()) + .setStatements(createNestedClassStatements(nestedMethodSettingsMemberVarExprs)) .build(); } - private static List createNestedClassStatements() { + private static List createNestedClassStatements( + Map nestedMethodSettingsMemberVarExprs) { List varDeclExprs = new ArrayList<>(); + + // Declare unaryMethodSettingsBuilders. Function varDeclFn = v -> v.toBuilder().setIsDecl(true).setScope(ScopeNode.PRIVATE).setIsFinal(true).build(); varDeclExprs.add(varDeclFn.apply(NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR)); + // Declare all the settings fields. + varDeclExprs.addAll( + nestedMethodSettingsMemberVarExprs.values().stream() + .map(v -> varDeclFn.apply(v)) + .collect(Collectors.toList())); + + // Declare the RETRYABLE_CODE_DEFNITIONS field. Function varStaticDeclFn = v -> v.toBuilder() @@ -1097,6 +1110,8 @@ private static List createNestedClassStatements() { .setIsFinal(true) .build(); varDeclExprs.add(varStaticDeclFn.apply(NESTED_RETRYABLE_CODE_DEFINITIONS_VAR_EXPR)); + + // Declare the RETRY_PARAM_DEFINITIONS field. varDeclExprs.add(varStaticDeclFn.apply(NESTED_RETRY_PARAM_DEFINITIONS_VAR_EXPR)); return varDeclExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList()); @@ -1299,22 +1314,35 @@ private static String getGrpcServiceStubTypeName(String serviceName) { return String.format(GRPC_SERVICE_STUB_PATTERN, JavaStyle.toUpperCamelCase(serviceName)); } - private static TypeNode getCallSettingsType(Method method, Map types) { + private static TypeNode getCallSettingsType( + Method method, Map types, final boolean isSettingsBuilder) { + Function typeMakerFn = + clz -> TypeNode.withReference(ConcreteReference.withClazz(clz)); // Default: No streaming. TypeNode callSettingsType = method.isPaged() - ? STATIC_TYPES.get("PagedCallSettings") - : STATIC_TYPES.get("UnaryCallSettings"); + ? typeMakerFn.apply( + isSettingsBuilder ? PagedCallSettings.Builder.class : PagedCallSettings.class) + : typeMakerFn.apply( + isSettingsBuilder ? UnaryCallSettings.Builder.class : UnaryCallSettings.class); // Streaming takes precendence over paging, as per the monolith's existing behavior. switch (method.stream()) { case SERVER: - callSettingsType = STATIC_TYPES.get("ServerStreamingCallSettings"); + callSettingsType = + typeMakerFn.apply( + isSettingsBuilder + ? ServerStreamingCallSettings.Builder.class + : ServerStreamingCallSettings.class); break; case CLIENT: // Fall through. case BIDI: - callSettingsType = STATIC_TYPES.get("StreamingCallSettings"); + callSettingsType = + typeMakerFn.apply( + isSettingsBuilder + ? StreamingCallSettings.Builder.class + : StreamingCallSettings.class); break; case NONE: // Fall through. @@ -1331,7 +1359,7 @@ private static TypeNode getCallSettingsType(Method method, Map return TypeNode.withReference(callSettingsType.reference().copyAndSetGenerics(generics)); } - private static TypeNode getOperationCallSettingsType(Method method) { + private static TypeNode getOperationCallSettingsType(Method method, boolean isSettingsBuilder) { Preconditions.checkState( method.hasLro(), String.format("Cannot get OperationCallSettings for non-LRO method %s", method.name())); @@ -1340,6 +1368,12 @@ private static TypeNode getOperationCallSettingsType(Method method) { generics.add(method.lro().responseType().reference()); generics.add(method.lro().metadataType().reference()); return TypeNode.withReference( - STATIC_TYPES.get("OperationCallSettings").reference().copyAndSetGenerics(generics)); + ConcreteReference.builder() + .setClazz( + isSettingsBuilder + ? OperationCallSettings.Builder.class + : OperationCallSettings.class) + .setGenerics(generics) + .build()); } } 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 78222adf5d..67133297f0 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 @@ -330,6 +330,25 @@ public void generateServiceClasses() { + " public static class Builder extends StubSettings.Builder {\n" + " private final ImmutableList>" + " unaryMethodSettingsBuilders;\n" + + " private final UnaryCallSettings.Builder" + + " echoSettings;\n" + + " private final ServerStreamingCallSettings.Builder" + + " expandSettings;\n" + + " private final StreamingCallSettings.Builder" + + " collectSettings;\n" + + " private final StreamingCallSettings.Builder" + + " chatSettings;\n" + + " private final StreamingCallSettings.Builder" + + " chatAgainSettings;\n" + + " private final PagedCallSettings.Builder<\n" + + " PagedExpandRequest, PagedExpandResponse, PagedExpandPagedResponse>\n" + + " pagedExpandSettings;\n" + + " private final UnaryCallSettings.Builder waitSettings;\n" + + " private final OperationCallSettings.Builder\n" + + " waitOperationSettings;\n" + + " private final UnaryCallSettings.Builder" + + " blockSettings;\n" + " private static final ImmutableMap>\n" + " RETRYABLE_CODE_DEFINITIONS;\n" + " private static final ImmutableMap" From dbc57359b23b9a22429d002f5e6bef64ac908f94 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 25 Aug 2020 17:59:33 -0700 Subject: [PATCH 06/24] feat: add first nested ctors to ServiceStubSettings codegen --- .../ServiceStubSettingsClassComposer.java | 179 +++++++++++++++++- .../ServiceStubSettingsClassComposerTest.java | 22 +++ 2 files changed, 199 insertions(+), 2 deletions(-) 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 311f6fa1dd..ce11685ae1 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 @@ -54,6 +54,7 @@ import com.google.api.generator.engine.ast.AnnotationNode; import com.google.api.generator.engine.ast.AnonymousClassExpr; 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.ConcreteReference; import com.google.api.generator.engine.ast.Expr; @@ -118,6 +119,7 @@ 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 SETTINGS_LITERAL = "Settings"; private static final String LEFT_BRACE = "{"; private static final String RIGHT_BRACE = "}"; @@ -285,11 +287,25 @@ private static List createPagingStaticAssignExprs( if (!method.isPaged()) { continue; } + // Find the repeated type. - Message pagedResponseMessage = - messageTypes.get(JavaStyle.toUpperCamelCase(method.outputType().reference().name())); + String pagedResponseMessageKey = + JavaStyle.toUpperCamelCase(method.outputType().reference().name()); + if (method.hasLro()) { + pagedResponseMessageKey = + JavaStyle.toUpperCamelCase(method.lro().responseType().reference().name()); + } + Message pagedResponseMessage = messageTypes.get(pagedResponseMessageKey); + Preconditions.checkNotNull( + pagedResponseMessage, + String.format( + "No method found for message type %s for method %s among %s", + pagedResponseMessageKey, method.name(), messageTypes.keySet())); TypeNode repeatedResponseType = null; for (Field field : pagedResponseMessage.fields()) { + Preconditions.checkState( + field != null, + String.format("Null field found for message %s", pagedResponseMessage.name())); if (field.isRepeated()) { // Field is currently a List-type. Preconditions.checkState( @@ -1082,6 +1098,7 @@ private static ClassDefinition createNestedBuilderClass( .setName(className) .setExtendsType(extendsType) .setStatements(createNestedClassStatements(nestedMethodSettingsMemberVarExprs)) + .setMethods(createNestedClassMethods(nestedMethodSettingsMemberVarExprs, types)) .build(); } @@ -1117,6 +1134,164 @@ private static List createNestedClassStatements( return varDeclExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList()); } + private static List createNestedClassMethods( + Map nestedMethodSettingsMemberVarExprs, Map types) { + List nestedClassMethods = new ArrayList<>(); + nestedClassMethods.addAll( + createNestedClassConstructorMethods(nestedMethodSettingsMemberVarExprs, types)); + + // TODO(miraleung): initDefaults(). + return nestedClassMethods; + } + + private static List createNestedClassConstructorMethods( + Map nestedMethodSettingsMemberVarExprs, Map types) { + TypeNode builderType = types.get(NESTED_BUILDER_CLASS_NAME); + + List ctorMethods = new ArrayList<>(); + + // First argument-less contructor. + ctorMethods.add( + MethodDefinition.constructorBuilder() + .setScope(ScopeNode.PROTECTED) + .setReturnType(builderType) + .setBody( + Arrays.asList( + ExprStatement.withExpr( + ReferenceConstructorExpr.thisBuilder() + .setType(builderType) + .setArguments( + CastExpr.builder() + .setType(STATIC_TYPES.get("ClientContext")) + .setExpr(ValueExpr.withValue(NullObjectValue.create())) + .build()) + .build()))) + .build()); + + // Second ctor that takes a clientContext argument. + VariableExpr clientContextVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setType(STATIC_TYPES.get("ClientContext")) + .setName("clientContext") + .build()); + Reference pagedSettingsBuilderRef = + ConcreteReference.withClazz(PagedCallSettings.Builder.class); + Reference unaryCallSettingsBuilderRef = + ConcreteReference.withClazz(UnaryCallSettings.Builder.class); + Function isUnaryCallSettingsBuilderFn = + t -> + t.reference() + .copyAndSetGenerics(ImmutableList.of()) + .equals(unaryCallSettingsBuilderRef); + Function isPagedCallSettingsBuilderFn = + t -> t.reference().copyAndSetGenerics(ImmutableList.of()).equals(pagedSettingsBuilderRef); + Function builderToCallSettingsFn = + t -> + TypeNode.withReference( + VaporReference.builder() + .setName(t.reference().enclosingClassName()) + .setPakkage(t.reference().pakkage()) + .build()); + List ctorBodyExprs = new ArrayList<>(); + ctorBodyExprs.add( + ReferenceConstructorExpr.superBuilder() + .setType(builderType) + .setArguments(clientContextVarExpr) + .build()); + ctorBodyExprs.addAll( + nestedMethodSettingsMemberVarExprs.entrySet().stream() + .map( + e -> { + // Name is fooBarSettings. + VariableExpr varExpr = e.getValue(); + TypeNode varType = varExpr.type(); + if (!isPagedCallSettingsBuilderFn.apply(varType)) { + boolean isUnaryCallSettings = isUnaryCallSettingsBuilderFn.apply(varType); + return AssignmentExpr.builder() + .setVariableExpr(varExpr) + .setValueExpr( + MethodInvocationExpr.builder() + .setStaticReferenceType( + builderToCallSettingsFn.apply(varExpr.type())) + .setMethodName( + isUnaryCallSettings + ? "newUnaryCallSettingsBuilder" + : "newBuilder") + .setReturnType(varExpr.type()) + .build()) + .build(); + } + String varName = e.getKey(); + Preconditions.checkState( + varName.endsWith(SETTINGS_LITERAL), + String.format("%s expected to end with \"Settings\"", varName)); + varName = varName.substring(0, varName.length() - SETTINGS_LITERAL.length()); + varName = + String.format( + PAGED_RESPONSE_FACTORY_PATTERN, JavaStyle.toUpperSnakeCase(varName)); + VariableExpr argVar = + VariableExpr.withVariable( + Variable.builder() + .setType(STATIC_TYPES.get("PagedListResponseFactory")) + .setName(varName) + .build()); + return AssignmentExpr.builder() + .setVariableExpr(varExpr) + .setValueExpr( + MethodInvocationExpr.builder() + .setStaticReferenceType(builderToCallSettingsFn.apply(varExpr.type())) + .setMethodName("newBuilder") + .setArguments(argVar) + .setReturnType(varExpr.type()) + .build()) + .build(); + }) + .collect(Collectors.toList())); + + ctorBodyExprs.add( + AssignmentExpr.builder() + .setVariableExpr(NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR) + .setValueExpr( + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("ImmutableList")) + .setGenerics( + NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR + .type() + .reference() + .generics()) + .setMethodName("of") + .setArguments( + nestedMethodSettingsMemberVarExprs.values().stream() + .filter( + v -> + isUnaryCallSettingsBuilderFn.apply(v.type()) + || isPagedCallSettingsBuilderFn.apply(v.type())) + .collect(Collectors.toList())) + .setReturnType(NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR.type()) + .build()) + .build()); + + ctorBodyExprs.add( + MethodInvocationExpr.builder() + .setMethodName("initDefaults") + .setArguments(ValueExpr.withValue(ThisObjectValue.withType(builderType))) + .build()); + + ctorMethods.add( + MethodDefinition.constructorBuilder() + .setScope(ScopeNode.PROTECTED) + .setReturnType(builderType) + .setArguments(clientContextVarExpr.toBuilder().setIsDecl(true).build()) + .setBody( + ctorBodyExprs.stream() + .map(e -> ExprStatement.withExpr(e)) + .collect(Collectors.toList())) + .build()); + + return ctorMethods; + } + private static Map createStaticTypes() { List concreteClazzes = Arrays.asList( 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 67133297f0..fbf5fd8f54 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 @@ -353,6 +353,28 @@ public void generateServiceClasses() { + " RETRYABLE_CODE_DEFINITIONS;\n" + " private static final ImmutableMap" + " RETRY_PARAM_DEFINITIONS;\n" + + "\n" + + " protected Builder() {\n" + + " this(((ClientContext) null));\n" + + " }\n" + + "\n" + + " protected Builder(ClientContext clientContext) {\n" + + " super(clientContext);\n" + + " echoSettings = UnaryCallSettings.newUnaryCallSettingsBuilder();\n" + + " expandSettings = ServerStreamingCallSettings.newBuilder();\n" + + " collectSettings = StreamingCallSettings.newBuilder();\n" + + " chatSettings = StreamingCallSettings.newBuilder();\n" + + " chatAgainSettings = StreamingCallSettings.newBuilder();\n" + + " pagedExpandSettings =" + + " PagedCallSettings.newBuilder(PAGED_EXPAND_PAGE_STR_FACT);\n" + + " waitSettings = UnaryCallSettings.newUnaryCallSettingsBuilder();\n" + + " waitOperationSettings = OperationCallSettings.newBuilder();\n" + + " blockSettings = UnaryCallSettings.newUnaryCallSettingsBuilder();\n" + + " unaryMethodSettingsBuilders =\n" + + " ImmutableList.>of(\n" + + " echoSettings, pagedExpandSettings, waitSettings, blockSettings);\n" + + " initDefaults(this);\n" + + " }\n" + " }\n" + "}\n"; } From 164c0b3134509c2e5c51400395f36997552df111 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 26 Aug 2020 15:35:31 -0700 Subject: [PATCH 07/24] feat: add GapicServiceConfig DS and processing --- .../api/generator/gapic/model/BUILD.bazel | 2 + .../gapic/model/GapicServiceConfig.java | 179 ++++++++++++++++++ .../generator/gapic/model/RetrySettings.java | 44 +++++ .../api/generator/gapic/model/BUILD.bazel | 10 + .../gapic/model/GapicServiceConfigTest.java | 148 +++++++++++++++ 5 files changed, 383 insertions(+) create mode 100644 src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java create mode 100644 src/main/java/com/google/api/generator/gapic/model/RetrySettings.java create mode 100644 src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java diff --git a/src/main/java/com/google/api/generator/gapic/model/BUILD.bazel b/src/main/java/com/google/api/generator/gapic/model/BUILD.bazel index e0106a3c27..e883369a61 100644 --- a/src/main/java/com/google/api/generator/gapic/model/BUILD.bazel +++ b/src/main/java/com/google/api/generator/gapic/model/BUILD.bazel @@ -11,6 +11,7 @@ java_library( ":model_files", ], deps = [ + "//:rpc_java_proto", "//:service_config_java_proto", "//src/main/java/com/google/api/generator:autovalue", "//src/main/java/com/google/api/generator/engine/ast", @@ -19,5 +20,6 @@ java_library( "@com_google_auto_value_auto_value_annotations//jar", "@com_google_code_findbugs_jsr305//jar", "@com_google_guava_guava//jar", + "@com_google_protobuf//:protobuf_java", ], ) diff --git a/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java b/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java new file mode 100644 index 0000000000..d15cfb529b --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java @@ -0,0 +1,179 @@ +// 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.model; + +import com.google.protobuf.Duration; +import com.google.rpc.Code; +import io.grpc.serviceconfig.MethodConfig; +import io.grpc.serviceconfig.MethodConfig.RetryPolicy; +import io.grpc.serviceconfig.ServiceConfig; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; + +public class GapicServiceConfig { + public static final RetryPolicy EMPTY_RETRY_POLICY = RetryPolicy.newBuilder().build(); + public static final Duration EMPTY_TIMEOUT = Duration.newBuilder().setSeconds(5).build(); + public static final List EMPTY_RETRY_CODES = Collections.emptyList(); + + private static final String NO_RETRY_CODES_NAME = "no_retry_codes"; + private static final String NO_RETRY_PARAMS_NAME = "no_retry_params"; + + private static final String NO_RETRY_CODES_PATTERN = "no_retry_%d_codes"; + private static final String NO_RETRY_PARAMS_PATTERN = "no_retry_%d_params"; + private static final String RETRY_CODES_PATTERN = "retry_policy_%d_codes"; + private static final String RETRY_PARAMS_PATTERN = "retry_policy_%d_params"; + + private final List methodConfigs; + private final Map methodConfigTable; + + private GapicServiceConfig( + List methodConfigs, Map methodConfigTable) { + this.methodConfigs = methodConfigs; + this.methodConfigTable = methodConfigTable; + } + + public static GapicServiceConfig create(ServiceConfig serviceConfig) { + // Keep this processing logic out of the constructor. + Map methodConfigTable = new HashMap<>(); + List methodConfigs = serviceConfig.getMethodConfigList(); + for (int i = 0; i < methodConfigs.size(); i++) { + MethodConfig methodConfig = methodConfigs.get(i); + for (MethodConfig.Name name : methodConfig.getNameList()) { + methodConfigTable.put(name, i); + } + } + + return new GapicServiceConfig(methodConfigs, methodConfigTable); + } + + public Map getAllRetrySettings(Service service) { + return service.methods().stream() + .collect( + Collectors.toMap( + m -> getRetryParamsName(service, m), + m -> RetrySettings.with(timeoutLookup(service, m), retryPolicyLookup(service, m)), + (r1, r2) -> r2)); + } + + public Map> getAllRetryCodes(Service service) { + return service.methods().stream() + .collect( + Collectors.toMap( + m -> getRetryCodeName(service, m), + m -> retryCodesLookup(service, m), + (l1, l2) -> l2)); + } + + public String getRetryCodeName(Service service, Method method) { + Optional retryPolicyIndexOpt = retryPolicyIndexLookup(service, method); + if (retryPolicyIndexOpt.isPresent()) { + return getRetryCodeName(retryPolicyIndexOpt.get()); + } + return NO_RETRY_CODES_NAME; + } + + public String getRetryParamsName(Service service, Method method) { + Optional retryPolicyIndexOpt = retryPolicyIndexLookup(service, method); + if (retryPolicyIndexOpt.isPresent()) { + return getRetryParamsName(retryPolicyIndexOpt.get()); + } + return NO_RETRY_PARAMS_NAME; + } + + private List retryCodesLookup(Service service, Method method) { + RetryPolicy retryPolicy = retryPolicyLookup(service, method); + if (retryPolicy.equals(EMPTY_RETRY_POLICY)) { + return Collections.emptyList(); + } + return retryPolicy.getRetryableStatusCodesList(); + } + + private RetryPolicy retryPolicyLookup(Service service, Method method) { + Optional retryPolicyIndexOpt = retryPolicyIndexLookup(service, method); + if (retryPolicyIndexOpt.isPresent()) { + MethodConfig methodConfig = methodConfigs.get(retryPolicyIndexOpt.get()); + return methodConfig.hasRetryPolicy() ? methodConfig.getRetryPolicy() : EMPTY_RETRY_POLICY; + } + return EMPTY_RETRY_POLICY; + } + + private Duration timeoutLookup(Service service, Method method) { + Optional retryPolicyIndexOpt = retryPolicyIndexLookup(service, method); + if (retryPolicyIndexOpt.isPresent()) { + MethodConfig methodConfig = methodConfigs.get(retryPolicyIndexOpt.get()); + return methodConfig.hasTimeout() ? methodConfig.getTimeout() : EMPTY_TIMEOUT; + } + return EMPTY_TIMEOUT; + } + + private Optional retryPolicyIndexLookup(Service service, Method method) { + MethodConfig.Name serviceMethodName = toName(service, method); + if (methodConfigTable.containsKey(serviceMethodName)) { + return Optional.of(methodConfigTable.get(serviceMethodName)); + } + MethodConfig.Name serviceName = toName(service); + if (methodConfigTable.containsKey(serviceName)) { + return Optional.of(methodConfigTable.get(serviceName)); + } + return Optional.empty(); + } + + /** + * @param codeIndex The index of the retryable code in the method config list. + * @return The canonical name to be used in the generated client library. + */ + private String getRetryCodeName(int methodConfigIndex) { + MethodConfig methodConfig = methodConfigs.get(methodConfigIndex); + if (!methodConfig.hasTimeout() && !methodConfig.hasRetryPolicy()) { + return NO_RETRY_CODES_NAME; + } + return String.format( + methodConfig.hasRetryPolicy() ? RETRY_CODES_PATTERN : NO_RETRY_CODES_PATTERN, + methodConfigIndex); + } + + /** + * @param retryParamsIndex The index of the retry params in the method config list. + * @return The canonical name to be used in the generated client library. + */ + private String getRetryParamsName(int methodConfigIndex) { + MethodConfig methodConfig = methodConfigs.get(methodConfigIndex); + if (!methodConfig.hasTimeout() && !methodConfig.hasRetryPolicy()) { + return NO_RETRY_PARAMS_NAME; + } + return String.format( + methodConfig.hasRetryPolicy() ? RETRY_PARAMS_PATTERN : NO_RETRY_PARAMS_PATTERN, + methodConfigIndex); + } + + private MethodConfig.Name toName(Service service) { + return MethodConfig.Name.newBuilder().setService(serviceToNameString(service)).build(); + } + + private MethodConfig.Name toName(Service service, Method method) { + return MethodConfig.Name.newBuilder() + .setService(serviceToNameString(service)) + .setMethod(method.name()) + .build(); + } + + private String serviceToNameString(Service service) { + return String.format("%s.%s", service.protoPakkage(), service.name()); + } +} diff --git a/src/main/java/com/google/api/generator/gapic/model/RetrySettings.java b/src/main/java/com/google/api/generator/gapic/model/RetrySettings.java new file mode 100644 index 0000000000..acb85487f3 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/model/RetrySettings.java @@ -0,0 +1,44 @@ +// 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.model; + +import com.google.auto.value.AutoValue; +import com.google.protobuf.Duration; +import io.grpc.serviceconfig.MethodConfig.RetryPolicy; + +@AutoValue +public abstract class RetrySettings { + public abstract Duration timeout(); + + public abstract RetryPolicy retryPolicy(); + + public static RetrySettings with(Duration timeout, RetryPolicy retryPolicy) { + return builder().setTimeout(timeout).setRetryPolicy(retryPolicy).build(); + } + + // Private. + static Builder builder() { + return new AutoValue_RetrySettings.Builder(); + } + + @AutoValue.Builder + abstract static class Builder { + abstract Builder setTimeout(Duration timeout); + + abstract Builder setRetryPolicy(RetryPolicy retryPolicy); + + abstract RetrySettings build(); + } +} diff --git a/src/test/java/com/google/api/generator/gapic/model/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/model/BUILD.bazel index 4373b2111f..637631b3f0 100644 --- a/src/test/java/com/google/api/generator/gapic/model/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/model/BUILD.bazel @@ -1,6 +1,7 @@ package(default_visibility = ["//visibility:public"]) TESTS = [ + "GapicServiceConfigTest", "MethodTest", ] @@ -12,10 +13,19 @@ filegroup( [java_test( name = test_name, srcs = ["{0}.java".format(test_name)], + data = [ + "//src/test/java/com/google/api/generator/gapic/testdata:service_config_files", + ], test_class = "com.google.api.generator.gapic.model.{0}".format(test_name), deps = [ + "//:rpc_java_proto", + "//:service_config_java_proto", "//src/main/java/com/google/api/generator:autovalue", "//src/main/java/com/google/api/generator/gapic/model", + "//src/main/java/com/google/api/generator/gapic/protoparser", + "//src/test/java/com/google/api/generator/gapic/testdata:showcase_java_proto", + "@com_google_protobuf//:protobuf_java", + "@com_google_protobuf//:protobuf_java_util", "@com_google_truth_truth//jar", "@junit_junit//jar", ], 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 new file mode 100644 index 0000000000..bf1f5dfd4a --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java @@ -0,0 +1,148 @@ +// 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.model; + +import static com.google.common.truth.Truth.assertThat; +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertTrue; + +import com.google.api.generator.gapic.protoparser.Parser; +import com.google.api.generator.gapic.protoparser.ServiceConfigParser; +import com.google.protobuf.Descriptors.FileDescriptor; +import com.google.protobuf.Descriptors.ServiceDescriptor; +import com.google.protobuf.util.Durations; +import com.google.rpc.Code; +import com.google.showcase.v1beta1.EchoOuterClass; +import io.grpc.serviceconfig.MethodConfig; +import io.grpc.serviceconfig.ServiceConfig; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import org.junit.Test; + +public class GapicServiceConfigTest { + private static final double EPSILON = 1e-4; + private static final String JSON_DIRECTORY = + "src/test/java/com/google/api/generator/gapic/testdata/"; + + @Test + public void serviceConfig_noConfigsFound() { + FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor(); + ServiceDescriptor echoServiceDescriptor = echoFileDescriptor.getServices().get(0); + Service service = parseService(echoFileDescriptor); + + String jsonFilename = "retrying_grpc_service_config.json"; + Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + Optional rawConfigOpt = ServiceConfigParser.parseFile(jsonPath.toString()); + assertTrue(rawConfigOpt.isPresent()); + + ServiceConfig rawConfig = rawConfigOpt.get(); + GapicServiceConfig serviceConfig = GapicServiceConfig.create(rawConfig); + + Map retrySettings = serviceConfig.getAllRetrySettings(service); + assertEquals(1, retrySettings.size()); + String retryParamsName = serviceConfig.getRetryParamsName(service, service.methods().get(0)); + assertEquals("no_retry_params", retryParamsName); + + assertEquals(GapicServiceConfig.EMPTY_TIMEOUT, retrySettings.get(retryParamsName).timeout()); + assertEquals( + GapicServiceConfig.EMPTY_RETRY_POLICY, retrySettings.get(retryParamsName).retryPolicy()); + + Map> retryCodes = serviceConfig.getAllRetryCodes(service); + assertEquals(1, retryCodes.size()); + assertEquals( + "no_retry_codes", serviceConfig.getRetryCodeName(service, service.methods().get(0))); + assertEquals(GapicServiceConfig.EMPTY_RETRY_CODES, retryCodes.get("no_retry_codes")); + } + + @Test + public void serviceConfig_basic() { + FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor(); + ServiceDescriptor echoServiceDescriptor = echoFileDescriptor.getServices().get(0); + Service service = parseService(echoFileDescriptor); + + String jsonFilename = "showcase_grpc_service_config.json"; + Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + Optional rawConfigOpt = ServiceConfigParser.parseFile(jsonPath.toString()); + assertTrue(rawConfigOpt.isPresent()); + + ServiceConfig rawConfig = rawConfigOpt.get(); + GapicServiceConfig serviceConfig = GapicServiceConfig.create(rawConfig); + + Map retrySettings = serviceConfig.getAllRetrySettings(service); + assertEquals(2, retrySettings.size()); + Map> retryCodes = serviceConfig.getAllRetryCodes(service); + assertEquals(2, retryCodes.size()); + + // Echo method has an explicitly-defined setting. + Method method = findMethod(service, "Echo"); + assertThat(method).isNotNull(); + String retryParamsName = serviceConfig.getRetryParamsName(service, method); + assertEquals("retry_policy_1_params", retryParamsName); + RetrySettings settings = retrySettings.get(retryParamsName); + assertThat(settings).isNotNull(); + assertEquals(10, settings.timeout().getSeconds()); + + MethodConfig.RetryPolicy retryPolicy = settings.retryPolicy(); + assertEquals(3, retryPolicy.getMaxAttempts()); + assertEquals(100, Durations.toMillis(retryPolicy.getInitialBackoff())); + assertEquals(3000, Durations.toMillis(retryPolicy.getMaxBackoff())); + assertEquals(2.0, retryPolicy.getBackoffMultiplier(), EPSILON); + + String retryCodeName = serviceConfig.getRetryCodeName(service, method); + assertEquals("retry_policy_1_codes", retryCodeName); + List retryCode = retryCodes.get(retryCodeName); + assertThat(retryCode).containsExactly(Code.UNAVAILABLE, Code.UNKNOWN); + + // Chat method defaults to the service-defined setting. + method = findMethod(service, "Chat"); + assertThat(method).isNotNull(); + retryParamsName = serviceConfig.getRetryParamsName(service, method); + assertEquals("no_retry_0_params", retryParamsName); + + settings = retrySettings.get(retryParamsName); + assertThat(settings).isNotNull(); + assertEquals(5, settings.timeout().getSeconds()); + assertEquals(GapicServiceConfig.EMPTY_RETRY_POLICY, settings.retryPolicy()); + + retryCodeName = serviceConfig.getRetryCodeName(service, method); + assertEquals("no_retry_0_codes", retryCodeName); + assertEquals(GapicServiceConfig.EMPTY_RETRY_CODES, retryCodes.get(retryCodeName)); + } + + private static Service parseService(FileDescriptor fileDescriptor) { + Map messageTypes = Parser.parseMessages(fileDescriptor); + Map resourceNames = Parser.parseResourceNames(fileDescriptor); + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService(fileDescriptor, messageTypes, resourceNames, outputResourceNames); + assertEquals(1, services.size()); + + return services.get(0); + } + + private static Method findMethod(Service service, String methodName) { + for (Method m : service.methods()) { + if (m.name().equals(methodName)) { + return m; + } + } + return null; + } +} From 37832039e935f8f9170ca8dad652567301837ed0 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 26 Aug 2020 15:50:13 -0700 Subject: [PATCH 08/24] feat: integrate GapicServiceConfig into GapicContext, Parser, Composer --- .../api/generator/gapic/composer/Composer.java | 6 +++--- .../ServiceStubSettingsClassComposer.java | 8 ++++---- .../api/generator/gapic/model/GapicContext.java | 5 ++--- .../api/generator/gapic/protoparser/Parser.java | 4 ++-- .../gapic/protoparser/ServiceConfigParser.java | 13 ++++++++++++- .../ServiceStubSettingsClassComposerTest.java | 6 +++--- .../gapic/model/GapicServiceConfigTest.java | 17 ++++++----------- 7 files changed, 32 insertions(+), 27 deletions(-) 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 f54150735c..c193519023 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 @@ -19,12 +19,12 @@ import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.GapicClass.Kind; import com.google.api.generator.gapic.model.GapicContext; +import com.google.api.generator.gapic.model.GapicServiceConfig; 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 io.grpc.serviceconfig.ServiceConfig; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -45,7 +45,7 @@ public static List composeServiceClasses(GapicContext context) { public static List generateServiceClasses( @Nonnull Service service, - @Nullable ServiceConfig serviceConfig, + @Nullable GapicServiceConfig serviceConfig, @Nonnull Map messageTypes) { List clazzes = new ArrayList<>(); clazzes.addAll(generateStubClasses(service, serviceConfig, messageTypes)); @@ -64,7 +64,7 @@ public static List generateResourceNameHelperClasses( } public static List generateStubClasses( - Service service, ServiceConfig serviceConfig, Map messageTypes) { + Service service, GapicServiceConfig serviceConfig, Map messageTypes) { List clazzes = new ArrayList<>(); clazzes.add(ServiceStubClassComposer.instance().generate(service, messageTypes)); clazzes.add( 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 ce11685ae1..d91a9987dd 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 @@ -80,6 +80,7 @@ import com.google.api.generator.engine.ast.VariableExpr; import com.google.api.generator.gapic.model.Field; import com.google.api.generator.gapic.model.GapicClass; +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.Service; @@ -91,7 +92,6 @@ import com.google.common.collect.Lists; import com.google.longrunning.Operation; import com.google.protobuf.Empty; -import io.grpc.serviceconfig.ServiceConfig; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -145,7 +145,7 @@ public static ServiceStubSettingsClassComposer instance() { } public GapicClass generate( - Service service, ServiceConfig serviceConfig, Map messageTypes) { + Service service, GapicServiceConfig serviceConfig, Map messageTypes) { String pakkage = String.format("%s.stub", service.pakkage()); Map types = createDynamicTypes(service, pakkage); Map methodSettingsMemberVarExprs = @@ -215,7 +215,7 @@ private static Map createMethodSettingsClassMemberVarExprs private static List createClassStatements( Service service, - ServiceConfig serviceConfig, + GapicServiceConfig serviceConfig, Map methodSettingsMemberVarExprs, Map messageTypes, Map types) { @@ -277,7 +277,7 @@ private static List createClassStatements( private static List createPagingStaticAssignExprs( Service service, - ServiceConfig serviceConfig, + GapicServiceConfig serviceConfig, Map messageTypes, Map types) { // TODO(miraleung): Add a test case for several such statements. diff --git a/src/main/java/com/google/api/generator/gapic/model/GapicContext.java b/src/main/java/com/google/api/generator/gapic/model/GapicContext.java index 0d894884b9..59dbc06806 100644 --- a/src/main/java/com/google/api/generator/gapic/model/GapicContext.java +++ b/src/main/java/com/google/api/generator/gapic/model/GapicContext.java @@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import io.grpc.serviceconfig.ServiceConfig; import java.util.List; import java.util.Map; import java.util.Set; @@ -37,7 +36,7 @@ public abstract class GapicContext { public abstract ImmutableSet helperResourceNames(); @Nullable - public abstract ServiceConfig serviceConfig(); + public abstract GapicServiceConfig serviceConfig(); public static Builder builder() { return new AutoValue_GapicContext.Builder(); @@ -53,7 +52,7 @@ public abstract static class Builder { public abstract Builder setHelperResourceNames(Set helperResourceNames); - public abstract Builder setServiceConfig(ServiceConfig serviceConfig); + public abstract Builder setServiceConfig(GapicServiceConfig serviceConfig); public abstract GapicContext 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 f03079882d..dd80c26b69 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 @@ -21,6 +21,7 @@ import com.google.api.generator.engine.ast.VaporReference; import com.google.api.generator.gapic.model.Field; import com.google.api.generator.gapic.model.GapicContext; +import com.google.api.generator.gapic.model.GapicServiceConfig; import com.google.api.generator.gapic.model.LongrunningOperation; import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.Method; @@ -46,7 +47,6 @@ import com.google.protobuf.Descriptors.MethodDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; -import io.grpc.serviceconfig.ServiceConfig; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -72,7 +72,7 @@ public GapicParserException(String errorMessage) { public static GapicContext parse(CodeGeneratorRequest request) { Optional serviceConfigPathOpt = PluginArgumentParser.parseJsonConfigPath(request); String serviceConfigPath = serviceConfigPathOpt.isPresent() ? serviceConfigPathOpt.get() : null; - Optional serviceConfigOpt = ServiceConfigParser.parseFile(serviceConfigPath); + Optional serviceConfigOpt = ServiceConfigParser.parse(serviceConfigPath); // TODO(miraleung): Actually parse the yaml file. Optional gapicYamlConfigPathOpt = diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java index 25152f3051..24bbe84716 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java @@ -14,6 +14,8 @@ package com.google.api.generator.gapic.protoparser; +import com.google.api.generator.gapic.model.GapicServiceConfig; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import com.google.protobuf.util.JsonFormat; import io.grpc.serviceconfig.ServiceConfig; @@ -23,7 +25,16 @@ import java.util.Optional; public class ServiceConfigParser { - public static Optional parseFile(String serviceConfigFilePath) { + public static Optional parse(String serviceConfigFilePath) { + Optional rawConfigOpt = parseFile(serviceConfigFilePath); + if (!rawConfigOpt.isPresent()) { + return Optional.empty(); + } + return Optional.of(GapicServiceConfig.create(rawConfigOpt.get())); + } + + @VisibleForTesting + static Optional parseFile(String serviceConfigFilePath) { if (Strings.isNullOrEmpty(serviceConfigFilePath)) { return Optional.empty(); } 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 fbf5fd8f54..42851e5797 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 @@ -19,6 +19,7 @@ import com.google.api.generator.engine.writer.JavaWriterVisitor; import com.google.api.generator.gapic.model.GapicClass; +import com.google.api.generator.gapic.model.GapicServiceConfig; import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.model.Service; @@ -27,7 +28,6 @@ import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; import com.google.showcase.v1beta1.EchoOuterClass; -import io.grpc.serviceconfig.ServiceConfig; import java.nio.file.Path; import java.nio.file.Paths; import java.util.HashSet; @@ -62,9 +62,9 @@ public void generateServiceClasses() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional configOpt = ServiceConfigParser.parseFile(jsonPath.toString()); + Optional configOpt = ServiceConfigParser.parse(jsonPath.toString()); assertTrue(configOpt.isPresent()); - ServiceConfig config = configOpt.get(); + GapicServiceConfig config = configOpt.get(); Service echoProtoService = services.get(0); GapicClass clazz = 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 bf1f5dfd4a..a59f1968f4 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 @@ -26,7 +26,6 @@ import com.google.rpc.Code; import com.google.showcase.v1beta1.EchoOuterClass; import io.grpc.serviceconfig.MethodConfig; -import io.grpc.serviceconfig.ServiceConfig; import java.nio.file.Path; import java.nio.file.Paths; import java.util.HashSet; @@ -49,11 +48,9 @@ public void serviceConfig_noConfigsFound() { String jsonFilename = "retrying_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional rawConfigOpt = ServiceConfigParser.parseFile(jsonPath.toString()); - assertTrue(rawConfigOpt.isPresent()); - - ServiceConfig rawConfig = rawConfigOpt.get(); - GapicServiceConfig serviceConfig = GapicServiceConfig.create(rawConfig); + Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + assertTrue(serviceConfigOpt.isPresent()); + GapicServiceConfig serviceConfig = serviceConfigOpt.get(); Map retrySettings = serviceConfig.getAllRetrySettings(service); assertEquals(1, retrySettings.size()); @@ -79,11 +76,9 @@ public void serviceConfig_basic() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional rawConfigOpt = ServiceConfigParser.parseFile(jsonPath.toString()); - assertTrue(rawConfigOpt.isPresent()); - - ServiceConfig rawConfig = rawConfigOpt.get(); - GapicServiceConfig serviceConfig = GapicServiceConfig.create(rawConfig); + Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + assertTrue(serviceConfigOpt.isPresent()); + GapicServiceConfig serviceConfig = serviceConfigOpt.get(); Map retrySettings = serviceConfig.getAllRetrySettings(service); assertEquals(2, retrySettings.size()); From 3984eaa3dda0fd1c9f5c0df8ab193615831c0a14 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 26 Aug 2020 16:41:03 -0700 Subject: [PATCH 09/24] feat: initial param block, RetrySettingsComposer, test --- .../gapic/composer/RetrySettingsComposer.java | 113 ++++++++++++++++ .../gapic/model/GapicServiceConfig.java | 7 +- .../api/generator/gapic/composer/BUILD.bazel | 1 + .../composer/RetrySettingsComposerTest.java | 125 ++++++++++++++++++ 4 files changed, 244 insertions(+), 2 deletions(-) create mode 100644 src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java create mode 100644 src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java diff --git a/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java new file mode 100644 index 0000000000..018b2288c7 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java @@ -0,0 +1,113 @@ +// 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.retrying.RetrySettings; +import com.google.api.generator.engine.ast.AssignmentExpr; +import com.google.api.generator.engine.ast.BlockStatement; +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.MethodInvocationExpr; +import com.google.api.generator.engine.ast.NullObjectValue; +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.GapicServiceConfig; +import com.google.api.generator.gapic.model.Service; +import com.google.common.collect.ImmutableMap; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import org.threeten.bp.Duration; + +public class RetrySettingsComposer { + private static final Map STATIC_TYPES = createStaticTypes(); + + public static BlockStatement createRetryParamDefinitionsBlock( + Service service, + GapicServiceConfig serviceConfig, + VariableExpr retryParamDefinitionsClassMemberVarExpr) { + List bodyExprs = new ArrayList<>(); + + TypeNode definitionsType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(ImmutableMap.Builder.class) + .setGenerics(retryParamDefinitionsClassMemberVarExpr.type().reference().generics()) + .build()); + VariableExpr definitionsVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(definitionsType).setName("definitions").build()); + VariableExpr settingsVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setType(STATIC_TYPES.get("RetrySettings")) + .setName("settings") + .build()); + + // Create the first two exprs. + bodyExprs.add( + AssignmentExpr.builder() + .setVariableExpr(definitionsVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr( + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("ImmutableMap")) + .setMethodName("builder") + .setReturnType(definitionsVarExpr.type()) + .build()) + .build()); + bodyExprs.add( + AssignmentExpr.builder() + .setVariableExpr(settingsVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr(ValueExpr.withValue(NullObjectValue.create())) + .build()); + + // Build the settings object for each config. + // TODO(miraleung): Fill this out. + + // Reassign the new settings. + bodyExprs.add( + AssignmentExpr.builder() + .setVariableExpr(retryParamDefinitionsClassMemberVarExpr) + .setValueExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(definitionsVarExpr) + .setMethodName("build") + .setReturnType(retryParamDefinitionsClassMemberVarExpr.type()) + .build()) + .build()); + + // Put everything together. + return BlockStatement.builder() + .setIsStatic(true) + .setBody( + bodyExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList())) + .build(); + } + + private static Map createStaticTypes() { + List concreteClazzes = + Arrays.asList(Duration.class, ImmutableMap.class, RetrySettings.class); + return concreteClazzes.stream() + .collect( + Collectors.toMap( + c -> c.getSimpleName(), + c -> TypeNode.withReference(ConcreteReference.withClazz(c)))); + } +} diff --git a/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java b/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java index d15cfb529b..f8303deaee 100644 --- a/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java +++ b/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java @@ -21,6 +21,7 @@ import io.grpc.serviceconfig.ServiceConfig; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -68,7 +69,8 @@ public Map getAllRetrySettings(Service service) { Collectors.toMap( m -> getRetryParamsName(service, m), m -> RetrySettings.with(timeoutLookup(service, m), retryPolicyLookup(service, m)), - (r1, r2) -> r2)); + (r1, r2) -> r2, + LinkedHashMap::new)); } public Map> getAllRetryCodes(Service service) { @@ -77,7 +79,8 @@ public Map> getAllRetryCodes(Service service) { Collectors.toMap( m -> getRetryCodeName(service, m), m -> retryCodesLookup(service, m), - (l1, l2) -> l2)); + (l1, l2) -> l2, + LinkedHashMap::new)); } public String getRetryCodeName(Service service, Method method) { 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 a0fc9e4a60..eff9a8202a 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 @@ -7,6 +7,7 @@ TESTS = [ "MockServiceClassComposerTest", "MockServiceImplClassComposerTest", "ResourceNameHelperClassComposerTest", + "RetrySettingsComposerTest", "ServiceClientClassComposerTest", "ServiceClientTestClassComposerTest", "ServiceSettingsClassComposerTest", 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 new file mode 100644 index 0000000000..2cc9f65206 --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java @@ -0,0 +1,125 @@ +// 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.BlockStatement; +import com.google.api.generator.engine.ast.ConcreteReference; +import com.google.api.generator.engine.ast.TypeNode; +import com.google.api.generator.engine.ast.VaporReference; +import com.google.api.generator.engine.ast.Variable; +import com.google.api.generator.engine.ast.VariableExpr; +import com.google.api.generator.engine.writer.JavaWriterVisitor; +import com.google.api.generator.gapic.model.GapicServiceConfig; +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.protoparser.Parser; +import com.google.api.generator.gapic.protoparser.ServiceConfigParser; +import com.google.common.collect.ImmutableMap; +import com.google.protobuf.Descriptors.FileDescriptor; +import com.google.protobuf.Descriptors.ServiceDescriptor; +import com.google.showcase.v1beta1.EchoOuterClass; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import org.junit.Before; +import org.junit.Test; + +public class RetrySettingsComposerTest { + private static final String JSON_DIRECTORY = + "src/test/java/com/google/api/generator/gapic/testdata/"; + private static final VariableExpr RETRY_PARAM_DEFINITIONS_VAR_EXPR = + createRetryParamDefinitionsVarExpr(); + + private JavaWriterVisitor writerVisitor; + + @Before + public void setUp() { + writerVisitor = new JavaWriterVisitor(); + } + + @Test + public void paramDefinitionsBlock_noConfigsFound() { + FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor(); + ServiceDescriptor echoServiceDescriptor = echoFileDescriptor.getServices().get(0); + Map messageTypes = Parser.parseMessages(echoFileDescriptor); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + assertEquals(1, services.size()); + + Service service = services.get(0); + + String jsonFilename = "retrying_grpc_service_config.json"; + Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + assertTrue(serviceConfigOpt.isPresent()); + GapicServiceConfig serviceConfig = serviceConfigOpt.get(); + + BlockStatement paramDefinitionsBlock = + RetrySettingsComposer.createRetryParamDefinitionsBlock( + service, serviceConfig, RETRY_PARAM_DEFINITIONS_VAR_EXPR); + + paramDefinitionsBlock.accept(writerVisitor); + String expected = + createLines( + "static {\n", + "ImmutableMap.Builder definitions = ImmutableMap.builder();\n", + "RetrySettings settings = null;\n", + "RETRY_PARAM_DEFINITIONS = definitions.build();\n", + "}\n"); + assertEquals(expected, writerVisitor.write()); + } + + @Test + public void paramDefinitionsBlock_basic() { + // TODO(miraleung): Fill this out. + } + + private static VariableExpr createRetryParamDefinitionsVarExpr() { + TypeNode retrySettingsType = + TypeNode.withReference( + VaporReference.builder() + .setPakkage("com.google.api.gax.retrying") + .setName("RetrySettings") + .build()); + TypeNode varType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(ImmutableMap.class) + .setGenerics( + Arrays.asList(TypeNode.STRING, retrySettingsType).stream() + .map(t -> t.reference()) + .collect(Collectors.toList())) + .build()); + return VariableExpr.withVariable( + Variable.builder().setType(varType).setName("RETRY_PARAM_DEFINITIONS").build()); + } + + 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); + } +} From a0ef19e6cc2664646afbbb47e42c61159ffe800e Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 26 Aug 2020 17:20:33 -0700 Subject: [PATCH 10/24] fix!: refactor GapicRetrySettings --- .../gapic/composer/RetrySettingsComposer.java | 3 +-- ...ySettings.java => GapicRetrySettings.java} | 27 +++++++++++-------- .../gapic/model/GapicServiceConfig.java | 25 +++++++++++++++-- .../gapic/model/GapicServiceConfigTest.java | 9 ++++--- 4 files changed, 46 insertions(+), 18 deletions(-) rename src/main/java/com/google/api/generator/gapic/model/{RetrySettings.java => GapicRetrySettings.java} (60%) diff --git a/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java index 018b2288c7..35cffa36ba 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java @@ -34,7 +34,6 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; -import org.threeten.bp.Duration; public class RetrySettingsComposer { private static final Map STATIC_TYPES = createStaticTypes(); @@ -103,7 +102,7 @@ public static BlockStatement createRetryParamDefinitionsBlock( private static Map createStaticTypes() { List concreteClazzes = - Arrays.asList(Duration.class, ImmutableMap.class, RetrySettings.class); + Arrays.asList(org.threeten.bp.Duration.class, ImmutableMap.class, RetrySettings.class); return concreteClazzes.stream() .collect( Collectors.toMap( diff --git a/src/main/java/com/google/api/generator/gapic/model/RetrySettings.java b/src/main/java/com/google/api/generator/gapic/model/GapicRetrySettings.java similarity index 60% rename from src/main/java/com/google/api/generator/gapic/model/RetrySettings.java rename to src/main/java/com/google/api/generator/gapic/model/GapicRetrySettings.java index acb85487f3..c6539b2088 100644 --- a/src/main/java/com/google/api/generator/gapic/model/RetrySettings.java +++ b/src/main/java/com/google/api/generator/gapic/model/GapicRetrySettings.java @@ -19,26 +19,31 @@ import io.grpc.serviceconfig.MethodConfig.RetryPolicy; @AutoValue -public abstract class RetrySettings { +public abstract class GapicRetrySettings { + public enum Kind { + NONE, // No retry policy and no timeout. + NO_RETRY, // No retry policy, timeout only. + FULL // Retry policy and timeout. + }; + public abstract Duration timeout(); public abstract RetryPolicy retryPolicy(); - public static RetrySettings with(Duration timeout, RetryPolicy retryPolicy) { - return builder().setTimeout(timeout).setRetryPolicy(retryPolicy).build(); - } + public abstract Kind kind(); - // Private. - static Builder builder() { - return new AutoValue_RetrySettings.Builder(); + public static Builder builder() { + return new AutoValue_GapicRetrySettings.Builder(); } @AutoValue.Builder - abstract static class Builder { - abstract Builder setTimeout(Duration timeout); + public abstract static class Builder { + public abstract Builder setTimeout(Duration timeout); + + public abstract Builder setRetryPolicy(RetryPolicy retryPolicy); - abstract Builder setRetryPolicy(RetryPolicy retryPolicy); + public abstract Builder setKind(Kind kind); - abstract RetrySettings build(); + public abstract GapicRetrySettings build(); } } diff --git a/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java b/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java index f8303deaee..c19e23bfdc 100644 --- a/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java +++ b/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java @@ -63,12 +63,33 @@ public static GapicServiceConfig create(ServiceConfig serviceConfig) { return new GapicServiceConfig(methodConfigs, methodConfigTable); } - public Map getAllRetrySettings(Service service) { + public Map getAllGapicRetrySettings(Service service) { return service.methods().stream() .collect( Collectors.toMap( m -> getRetryParamsName(service, m), - m -> RetrySettings.with(timeoutLookup(service, m), retryPolicyLookup(service, m)), + m -> { + GapicRetrySettings.Kind kind = GapicRetrySettings.Kind.FULL; + Optional retryPolicyIndexOpt = retryPolicyIndexLookup(service, m); + if (!retryPolicyIndexOpt.isPresent()) { + kind = GapicRetrySettings.Kind.NONE; + } else { + MethodConfig methodConfig = methodConfigs.get(retryPolicyIndexOpt.get()); + if (!methodConfig.hasTimeout() && !methodConfig.hasRetryPolicy()) { + kind = GapicRetrySettings.Kind.NONE; + } else { + kind = + methodConfig.hasRetryPolicy() + ? GapicRetrySettings.Kind.FULL + : GapicRetrySettings.Kind.NO_RETRY; + } + } + return GapicRetrySettings.builder() + .setTimeout(timeoutLookup(service, m)) + .setRetryPolicy(retryPolicyLookup(service, m)) + .setKind(kind) + .build(); + }, (r1, r2) -> r2, LinkedHashMap::new)); } 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 a59f1968f4..dbeb6c8e92 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 @@ -52,7 +52,7 @@ public void serviceConfig_noConfigsFound() { assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); - Map retrySettings = serviceConfig.getAllRetrySettings(service); + Map retrySettings = serviceConfig.getAllGapicRetrySettings(service); assertEquals(1, retrySettings.size()); String retryParamsName = serviceConfig.getRetryParamsName(service, service.methods().get(0)); assertEquals("no_retry_params", retryParamsName); @@ -60,6 +60,7 @@ public void serviceConfig_noConfigsFound() { assertEquals(GapicServiceConfig.EMPTY_TIMEOUT, retrySettings.get(retryParamsName).timeout()); assertEquals( GapicServiceConfig.EMPTY_RETRY_POLICY, retrySettings.get(retryParamsName).retryPolicy()); + assertEquals(GapicRetrySettings.Kind.NONE, retrySettings.get(retryParamsName).kind()); Map> retryCodes = serviceConfig.getAllRetryCodes(service); assertEquals(1, retryCodes.size()); @@ -80,7 +81,7 @@ public void serviceConfig_basic() { assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); - Map retrySettings = serviceConfig.getAllRetrySettings(service); + Map retrySettings = serviceConfig.getAllGapicRetrySettings(service); assertEquals(2, retrySettings.size()); Map> retryCodes = serviceConfig.getAllRetryCodes(service); assertEquals(2, retryCodes.size()); @@ -90,9 +91,10 @@ public void serviceConfig_basic() { assertThat(method).isNotNull(); String retryParamsName = serviceConfig.getRetryParamsName(service, method); assertEquals("retry_policy_1_params", retryParamsName); - RetrySettings settings = retrySettings.get(retryParamsName); + GapicRetrySettings settings = retrySettings.get(retryParamsName); assertThat(settings).isNotNull(); assertEquals(10, settings.timeout().getSeconds()); + assertEquals(GapicRetrySettings.Kind.FULL, settings.kind()); MethodConfig.RetryPolicy retryPolicy = settings.retryPolicy(); assertEquals(3, retryPolicy.getMaxAttempts()); @@ -115,6 +117,7 @@ public void serviceConfig_basic() { assertThat(settings).isNotNull(); assertEquals(5, settings.timeout().getSeconds()); assertEquals(GapicServiceConfig.EMPTY_RETRY_POLICY, settings.retryPolicy()); + assertEquals(GapicRetrySettings.Kind.NO_RETRY, settings.kind()); retryCodeName = serviceConfig.getRetryCodeName(service, method); assertEquals("no_retry_0_codes", retryCodeName); From 74dbcbb630b5125b99faad69b42c6b3248c52399 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 27 Aug 2020 00:15:17 -0700 Subject: [PATCH 11/24] fix: recognize 1. or .1 double patterns --- .../com/google/api/generator/engine/lexicon/Literal.java | 7 +++++-- .../google/api/generator/engine/lexicon/LiteralTest.java | 5 +++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/lexicon/Literal.java b/src/main/java/com/google/api/generator/engine/lexicon/Literal.java index be1e3c9bca..fb97f9fd0f 100644 --- a/src/main/java/com/google/api/generator/engine/lexicon/Literal.java +++ b/src/main/java/com/google/api/generator/engine/lexicon/Literal.java @@ -26,8 +26,9 @@ public class Literal { private static final Pattern LONG_PATTERN = Pattern.compile("^[0-9]+[Ll]?$"); private static final Pattern FLOAT_PATTERN = Pattern.compile("^[0-9]+([fF]|(\\.(([0-9]+[fF])|[fF])))?$"); - private static final Pattern DOUBLE_PATTERN = + private static final Pattern DOUBLE_PATTERN_ONE = Pattern.compile("^[0-9]+(\\.[0-9]+)?(\\.?[eE]\\-?[0-9]+)$"); + private static final Pattern DOUBLE_PATTERN_TWO = Pattern.compile("^\\d*\\.\\d+$|^\\d+\\.\\d*$"); public static boolean isBooleanLiteral(String str) { return str.equals(BOOLEAN_TRUE) || str.equals(BOOLEAN_FALSE); @@ -46,7 +47,9 @@ public static boolean isFloatLiteral(String str) { } public static boolean isDoubleLiteral(String str) { - return isFloatLiteral(str) || DOUBLE_PATTERN.matcher(str).matches(); + return isFloatLiteral(str) + || DOUBLE_PATTERN_ONE.matcher(str).matches() + || DOUBLE_PATTERN_TWO.matcher(str).matches(); } public static boolean isNullLiteral(String str) { diff --git a/src/test/java/com/google/api/generator/engine/lexicon/LiteralTest.java b/src/test/java/com/google/api/generator/engine/lexicon/LiteralTest.java index b278dc98cb..ac22cbde6e 100644 --- a/src/test/java/com/google/api/generator/engine/lexicon/LiteralTest.java +++ b/src/test/java/com/google/api/generator/engine/lexicon/LiteralTest.java @@ -55,6 +55,8 @@ public void floatDetected() { assertThat(Literal.isFloatLiteral("123")).isTrue(); assertThat(Literal.isFloatLiteral("123f")).isTrue(); assertThat(Literal.isFloatLiteral("123.")).isFalse(); + assertThat(Literal.isFloatLiteral("0.01")).isFalse(); + assertThat(Literal.isFloatLiteral(".01")).isFalse(); assertThat(Literal.isFloatLiteral("123.f")).isTrue(); assertThat(Literal.isFloatLiteral("123.F")).isTrue(); assertThat(Literal.isFloatLiteral("123.234F")).isTrue(); @@ -65,6 +67,9 @@ public void floatDetected() { @Test public void doubleDetected() { assertThat(Literal.isDoubleLiteral("123")).isTrue(); + assertThat(Literal.isDoubleLiteral("0.01")).isTrue(); + assertThat(Literal.isDoubleLiteral(".01")).isTrue(); + assertThat(Literal.isDoubleLiteral("123.0")).isTrue(); assertThat(Literal.isDoubleLiteral("123f")).isTrue(); assertThat(Literal.isDoubleLiteral("123E-2")).isTrue(); assertThat(Literal.isDoubleLiteral("123.134E-2")).isTrue(); From 34285db716f6631c7d8cd18cfb222e059ef15741 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 27 Aug 2020 00:19:08 -0700 Subject: [PATCH 12/24] feat: support BlockStatement in ClassDef stmts --- .../generator/engine/ast/ClassDefinition.java | 38 ++++++++++--------- .../engine/ast/ClassDefinitionTest.java | 28 ++++++++++++++ 2 files changed, 48 insertions(+), 18 deletions(-) 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 65475b8d15..2a5fd9b685 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 @@ -165,24 +165,26 @@ public ClassDefinition build() { for (Statement statement : classDef.statements()) { // TODO(xiaozhenliu): Add CommentStatement check here. Preconditions.checkState( - statement instanceof ExprStatement, - "Class statement type must be either an expression or comment statement"); - Expr expr = ((ExprStatement) statement).expression(); - if (expr instanceof VariableExpr) { - VariableExpr variableExpr = (VariableExpr) expr; - Preconditions.checkState( - variableExpr.isDecl(), "Class expression variable statements must be declarations"); - Preconditions.checkState( - !variableExpr.scope().equals(ScopeNode.LOCAL), - "Class variable statement cannot have a local scope"); - } else { - Preconditions.checkState( - expr instanceof AssignmentExpr, - "Class expression statement must be assignment or variable declaration"); - VariableExpr variableExpr = ((AssignmentExpr) expr).variableExpr(); - Preconditions.checkState( - !variableExpr.scope().equals(ScopeNode.LOCAL), - "Class variable in assignment statement cannot have a local scope"); + 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(); + if (expr instanceof VariableExpr) { + VariableExpr variableExpr = (VariableExpr) expr; + Preconditions.checkState( + variableExpr.isDecl(), "Class expression variable statements must be declarations"); + Preconditions.checkState( + !variableExpr.scope().equals(ScopeNode.LOCAL), + "Class variable statement cannot have a local scope"); + } else { + Preconditions.checkState( + expr instanceof AssignmentExpr, + "Class expression statement must be assignment or variable declaration"); + VariableExpr variableExpr = ((AssignmentExpr) expr).variableExpr(); + Preconditions.checkState( + !variableExpr.scope().equals(ScopeNode.LOCAL), + "Class variable in assignment statement cannot have a local scope"); + } } } diff --git a/src/test/java/com/google/api/generator/engine/ast/ClassDefinitionTest.java b/src/test/java/com/google/api/generator/engine/ast/ClassDefinitionTest.java index d2a0db7633..b00da11d2a 100644 --- a/src/test/java/com/google/api/generator/engine/ast/ClassDefinitionTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/ClassDefinitionTest.java @@ -44,6 +44,34 @@ public void validClassDefinition_basicWithCommentsAndHeader() { // No exception thrown, we're good. } + @Test + public void validClassDefinition_exprAndBlockStatements() { + ClassDefinition.builder() + .setName("LibraryServiceStub") + .setIsNested(true) + .setScope(ScopeNode.PUBLIC) + .setStatements( + Arrays.asList( + BlockStatement.builder().setIsStatic(true).build(), + ExprStatement.withExpr( + AssignmentExpr.builder() + .setVariableExpr( + VariableExpr.builder() + .setIsDecl(true) + .setScope(ScopeNode.PRIVATE) + .setVariable( + Variable.builder().setType(TypeNode.INT).setName("x").build()) + .build()) + .setValueExpr( + MethodInvocationExpr.builder() + .setMethodName("foobar") + .setReturnType(TypeNode.INT) + .build()) + .build()))) + .build(); + // No exception thrown, we're good. + } + @Test public void validClassDefinition_nestedBasic() { ClassDefinition.builder() From 2fc9d0e6193bded5cf39b05eef631d2efc5e24cd Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 27 Aug 2020 00:19:59 -0700 Subject: [PATCH 13/24] feat: add params block to ServiceStubSettings codegen --- .../api/generator/gapic/composer/BUILD.bazel | 2 + .../gapic/composer/RetrySettingsComposer.java | 142 +++++++++++++++++- .../ServiceStubSettingsClassComposer.java | 33 ++-- .../composer/RetrySettingsComposerTest.java | 39 ++++- .../ServiceStubSettingsClassComposerTest.java | 27 ++++ 5 files changed, 232 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/BUILD.bazel b/src/main/java/com/google/api/generator/gapic/composer/BUILD.bazel index d1847bc155..21dc25a810 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/BUILD.bazel +++ b/src/main/java/com/google/api/generator/gapic/composer/BUILD.bazel @@ -23,6 +23,8 @@ java_library( "@com_google_api_gax_java//jar", "@com_google_code_findbugs_jsr305//jar", "@com_google_guava_guava//jar", + "@com_google_protobuf//:protobuf_java", + "@com_google_protobuf//:protobuf_java_util", "@com_google_protobuf//java/core", "@io_grpc_java//api", "@io_grpc_java//protobuf", diff --git a/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java index 35cffa36ba..7d43a096e9 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java @@ -22,17 +22,25 @@ import com.google.api.generator.engine.ast.ExprStatement; import com.google.api.generator.engine.ast.MethodInvocationExpr; import com.google.api.generator.engine.ast.NullObjectValue; +import com.google.api.generator.engine.ast.PrimitiveValue; +import com.google.api.generator.engine.ast.StringObjectValue; 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.GapicRetrySettings; import com.google.api.generator.gapic.model.GapicServiceConfig; import com.google.api.generator.gapic.model.Service; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; +import com.google.protobuf.Duration; +import com.google.protobuf.util.Durations; +import io.grpc.serviceconfig.MethodConfig.RetryPolicy; import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.function.Function; import java.util.stream.Collectors; public class RetrySettingsComposer { @@ -78,7 +86,15 @@ public static BlockStatement createRetryParamDefinitionsBlock( .build()); // Build the settings object for each config. - // TODO(miraleung): Fill this out. + for (Map.Entry settingsEntry : + serviceConfig.getAllGapicRetrySettings(service).entrySet()) { + bodyExprs.addAll( + createRetrySettingsExprs( + settingsEntry.getKey(), + settingsEntry.getValue(), + settingsVarExpr, + definitionsVarExpr)); + } // Reassign the new settings. bodyExprs.add( @@ -100,6 +116,130 @@ public static BlockStatement createRetryParamDefinitionsBlock( .build(); } + private static List createRetrySettingsExprs( + String settingsName, + GapicRetrySettings settings, + VariableExpr settingsVarExpr, + VariableExpr definitionsVarExpr) { + Function durationToMillisValExprFn = + d -> + ValueExpr.withValue( + PrimitiveValue.builder() + .setType(TypeNode.LONG) + .setValue(String.format("%dL", Durations.toMillis(d))) + .build()); + Function floatToValExprFn = + f -> + ValueExpr.withValue( + PrimitiveValue.builder() + .setType(TypeNode.DOUBLE) + .setValue(String.format("%.1f", f)) + .build()); + Function durationMillisMethodFn = + v -> + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("Duration")) + .setMethodName("ofMillis") + .setArguments(v) + .build(); + + Expr settingsBuilderExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("RetrySettings")) + .setMethodName("newBuilder") + .build(); + + RetryPolicy retryPolicy = settings.retryPolicy(); + if (settings.kind().equals(GapicRetrySettings.Kind.FULL)) { + Preconditions.checkState( + retryPolicy.hasInitialBackoff(), + String.format("initialBackoff not found for setting %s", settingsName)); + settingsBuilderExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(settingsBuilderExpr) + .setMethodName("setInitialRetryDelay") + .setArguments( + durationMillisMethodFn.apply( + durationToMillisValExprFn.apply(retryPolicy.getInitialBackoff()))) + .build(); + + settingsBuilderExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(settingsBuilderExpr) + .setMethodName("setRetryDelayMultiplier") + .setArguments(floatToValExprFn.apply(retryPolicy.getBackoffMultiplier())) + .build(); + + Preconditions.checkState( + retryPolicy.hasMaxBackoff(), + String.format("maxBackoff not found for setting %s", settingsName)); + settingsBuilderExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(settingsBuilderExpr) + .setMethodName("setMaxRetryDelay") + .setArguments( + durationMillisMethodFn.apply( + durationToMillisValExprFn.apply(retryPolicy.getMaxBackoff()))) + .build(); + } + + if (!settings.kind().equals(GapicRetrySettings.Kind.NONE)) { + settingsBuilderExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(settingsBuilderExpr) + .setMethodName("setInitialRpcTimeout") + .setArguments( + durationMillisMethodFn.apply(durationToMillisValExprFn.apply(settings.timeout()))) + .build(); + } + + // This will always be done, no matter the type of the retry settings object. + settingsBuilderExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(settingsBuilderExpr) + .setMethodName("setRpcTimeoutMultiplier") + .setArguments( + ValueExpr.withValue( + PrimitiveValue.builder().setType(TypeNode.DOUBLE).setValue("1.0").build())) + .build(); + + if (!settings.kind().equals(GapicRetrySettings.Kind.NONE)) { + for (String setterMethodName : Arrays.asList("setMaxRpcTimeout", "setTotalTimeout")) { + settingsBuilderExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(settingsBuilderExpr) + .setMethodName(setterMethodName) + .setArguments( + durationMillisMethodFn.apply( + durationToMillisValExprFn.apply(settings.timeout()))) + .build(); + } + } + + settingsBuilderExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(settingsBuilderExpr) + .setMethodName("build") + .setReturnType(settingsVarExpr.type()) + .build(); + + Expr settingsAssignExpr = + AssignmentExpr.builder() + .setVariableExpr(settingsVarExpr) + .setValueExpr(settingsBuilderExpr) + .build(); + + Expr definitionsPutExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(definitionsVarExpr) + .setMethodName("put") + .setArguments( + ValueExpr.withValue(StringObjectValue.withValue(settingsName)), settingsVarExpr) + .build(); + + return Arrays.asList(settingsAssignExpr, definitionsPutExpr); + } + private static Map createStaticTypes() { List concreteClazzes = Arrays.asList(org.threeten.bp.Duration.class, ImmutableMap.class, RetrySettings.class); 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 d91a9987dd..8e35e40ef9 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 @@ -163,7 +163,8 @@ public GapicClass generate( createClassStatements( service, serviceConfig, methodSettingsMemberVarExprs, messageTypes, types)) .setMethods(createClassMethods(service, methodSettingsMemberVarExprs, types)) - .setNestedClasses(Arrays.asList(createNestedBuilderClass(service, types))) + .setNestedClasses( + Arrays.asList(createNestedBuilderClass(service, serviceConfig, types))) .build(); return GapicClass.create(GapicClass.Kind.STUB, classDef); } @@ -1069,7 +1070,7 @@ private static MethodDefinition createClassConstructor( } private static ClassDefinition createNestedBuilderClass( - Service service, Map types) { + Service service, GapicServiceConfig serviceConfig, Map types) { String thisClassName = getThisClassName(service.name()); TypeNode outerThisClassType = types.get(thisClassName); @@ -1097,22 +1098,25 @@ private static ClassDefinition createNestedBuilderClass( .setIsStatic(true) .setName(className) .setExtendsType(extendsType) - .setStatements(createNestedClassStatements(nestedMethodSettingsMemberVarExprs)) + .setStatements( + createNestedClassStatements(service, serviceConfig, nestedMethodSettingsMemberVarExprs)) .setMethods(createNestedClassMethods(nestedMethodSettingsMemberVarExprs, types)) .build(); } private static List createNestedClassStatements( + Service service, + GapicServiceConfig serviceConfig, Map nestedMethodSettingsMemberVarExprs) { - List varDeclExprs = new ArrayList<>(); + List exprs = new ArrayList<>(); // Declare unaryMethodSettingsBuilders. Function varDeclFn = v -> v.toBuilder().setIsDecl(true).setScope(ScopeNode.PRIVATE).setIsFinal(true).build(); - varDeclExprs.add(varDeclFn.apply(NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR)); + exprs.add(varDeclFn.apply(NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR)); // Declare all the settings fields. - varDeclExprs.addAll( + exprs.addAll( nestedMethodSettingsMemberVarExprs.values().stream() .map(v -> varDeclFn.apply(v)) .collect(Collectors.toList())); @@ -1126,12 +1130,23 @@ private static List createNestedClassStatements( .setIsStatic(true) .setIsFinal(true) .build(); - varDeclExprs.add(varStaticDeclFn.apply(NESTED_RETRYABLE_CODE_DEFINITIONS_VAR_EXPR)); + Function exprStatementFn = e -> ExprStatement.withExpr(e); + + List statements = new ArrayList<>(); + statements.addAll( + exprs.stream().map(e -> exprStatementFn.apply(e)).collect(Collectors.toList())); + statements.add( + exprStatementFn.apply((varStaticDeclFn.apply(NESTED_RETRYABLE_CODE_DEFINITIONS_VAR_EXPR)))); // Declare the RETRY_PARAM_DEFINITIONS field. - varDeclExprs.add(varStaticDeclFn.apply(NESTED_RETRY_PARAM_DEFINITIONS_VAR_EXPR)); + statements.add( + exprStatementFn.apply(varStaticDeclFn.apply(NESTED_RETRY_PARAM_DEFINITIONS_VAR_EXPR))); + + statements.add( + RetrySettingsComposer.createRetryParamDefinitionsBlock( + service, serviceConfig, NESTED_RETRY_PARAM_DEFINITIONS_VAR_EXPR)); - return varDeclExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList()); + return statements; } private static List createNestedClassMethods( 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 2cc9f65206..eafc028566 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 @@ -88,6 +88,8 @@ public void paramDefinitionsBlock_noConfigsFound() { "static {\n", "ImmutableMap.Builder definitions = ImmutableMap.builder();\n", "RetrySettings settings = null;\n", + "settings = RetrySettings.newBuilder().setRpcTimeoutMultiplier(1.0).build();\n", + "definitions.put(\"no_retry_params\", settings);\n", "RETRY_PARAM_DEFINITIONS = definitions.build();\n", "}\n"); assertEquals(expected, writerVisitor.write()); @@ -95,7 +97,42 @@ public void paramDefinitionsBlock_noConfigsFound() { @Test public void paramDefinitionsBlock_basic() { - // TODO(miraleung): Fill this out. + FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor(); + ServiceDescriptor echoServiceDescriptor = echoFileDescriptor.getServices().get(0); + Map messageTypes = Parser.parseMessages(echoFileDescriptor); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + assertEquals(1, services.size()); + + Service service = services.get(0); + + String jsonFilename = "showcase_grpc_service_config.json"; + Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + assertTrue(serviceConfigOpt.isPresent()); + GapicServiceConfig serviceConfig = serviceConfigOpt.get(); + + BlockStatement paramDefinitionsBlock = + RetrySettingsComposer.createRetryParamDefinitionsBlock( + service, serviceConfig, RETRY_PARAM_DEFINITIONS_VAR_EXPR); + + paramDefinitionsBlock.accept(writerVisitor); + String expected = + createLines( + "static {\n", + "ImmutableMap.Builder definitions = ImmutableMap.builder();\n", + "RetrySettings settings = null;\n", + "settings =" + + " RetrySettings.newBuilder().setInitialRetryDelay(Duration.ofMillis(100L)).setRetryDelayMultiplier(2.0).setMaxRetryDelay(Duration.ofMillis(3000L)).setInitialRpcTimeout(Duration.ofMillis(10000L)).setRpcTimeoutMultiplier(1.0).setMaxRpcTimeout(Duration.ofMillis(10000L)).setTotalTimeout(Duration.ofMillis(10000L)).build();\n", + "definitions.put(\"retry_policy_1_params\", settings);\n", + "settings =" + + " RetrySettings.newBuilder().setInitialRpcTimeout(Duration.ofMillis(5000L)).setRpcTimeoutMultiplier(1.0).setMaxRpcTimeout(Duration.ofMillis(5000L)).setTotalTimeout(Duration.ofMillis(5000L)).build();\n", + "definitions.put(\"no_retry_0_params\", settings);\n", + "RETRY_PARAM_DEFINITIONS = definitions.build();\n", + "}\n"); + assertEquals(expected, writerVisitor.write()); } private static VariableExpr createRetryParamDefinitionsVarExpr() { 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 42851e5797..ced1b897c7 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 @@ -124,6 +124,7 @@ public void generateServiceClasses() { + "import java.util.List;\n" + "import java.util.Objects;\n" + "import javax.annotation.Generated;\n" + + "import org.threeten.bp.Duration;\n" + "\n" + "@BetaApi\n" + "@Generated(\"by gapic-generator-java\")\n" @@ -354,6 +355,32 @@ public void generateServiceClasses() { + " private static final ImmutableMap" + " RETRY_PARAM_DEFINITIONS;\n" + "\n" + + " static {\n" + + " ImmutableMap.Builder definitions =" + + " ImmutableMap.builder();\n" + + " RetrySettings settings = null;\n" + + " settings =\n" + + " RetrySettings.newBuilder()\n" + + " .setInitialRetryDelay(Duration.ofMillis(100L))\n" + + " .setRetryDelayMultiplier(2.0)\n" + + " .setMaxRetryDelay(Duration.ofMillis(3000L))\n" + + " .setInitialRpcTimeout(Duration.ofMillis(10000L))\n" + + " .setRpcTimeoutMultiplier(1.0)\n" + + " .setMaxRpcTimeout(Duration.ofMillis(10000L))\n" + + " .setTotalTimeout(Duration.ofMillis(10000L))\n" + + " .build();\n" + + " definitions.put(\"retry_policy_1_params\", settings);\n" + + " settings =\n" + + " RetrySettings.newBuilder()\n" + + " .setInitialRpcTimeout(Duration.ofMillis(5000L))\n" + + " .setRpcTimeoutMultiplier(1.0)\n" + + " .setMaxRpcTimeout(Duration.ofMillis(5000L))\n" + + " .setTotalTimeout(Duration.ofMillis(5000L))\n" + + " .build();\n" + + " definitions.put(\"no_retry_0_params\", settings);\n" + + " RETRY_PARAM_DEFINITIONS = definitions.build();\n" + + " }\n" + + "\n" + " protected Builder() {\n" + " this(((ClientContext) null));\n" + " }\n" From f6727d8adb6dc0ec1971b2b8c2094f3d59101b84 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 27 Aug 2020 11:22:00 -0700 Subject: [PATCH 14/24] feat: add codes def to ServiceStubSettings codegen --- .../api/generator/gapic/composer/BUILD.bazel | 1 + .../gapic/composer/RetrySettingsComposer.java | 101 +++++++++++++++++- .../ServiceStubSettingsClassComposer.java | 7 +- .../api/generator/gapic/composer/BUILD.bazel | 2 + .../composer/RetrySettingsComposerTest.java | 99 +++++++++++++++++ .../ServiceStubSettingsClassComposerTest.java | 16 +++ 6 files changed, 224 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/BUILD.bazel b/src/main/java/com/google/api/generator/gapic/composer/BUILD.bazel index 21dc25a810..c9be270e28 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/BUILD.bazel +++ b/src/main/java/com/google/api/generator/gapic/composer/BUILD.bazel @@ -13,6 +13,7 @@ java_library( deps = [ "//:longrunning_java_proto", "//:monitored_resource_java_proto", + "//:rpc_java_proto", "//:service_config_java_proto", "//src/main/java/com/google/api/generator/engine/ast", "//src/main/java/com/google/api/generator/gapic:status_java_proto", diff --git a/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java index 7d43a096e9..612945c8ca 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java @@ -15,9 +15,11 @@ package com.google.api.generator.gapic.composer; import com.google.api.gax.retrying.RetrySettings; +import com.google.api.gax.rpc.StatusCode; import com.google.api.generator.engine.ast.AssignmentExpr; import com.google.api.generator.engine.ast.BlockStatement; import com.google.api.generator.engine.ast.ConcreteReference; +import com.google.api.generator.engine.ast.EnumRefExpr; import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.ExprStatement; import com.google.api.generator.engine.ast.MethodInvocationExpr; @@ -33,8 +35,11 @@ import com.google.api.generator.gapic.model.Service; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Lists; import com.google.protobuf.Duration; import com.google.protobuf.util.Durations; +import com.google.rpc.Code; import io.grpc.serviceconfig.MethodConfig.RetryPolicy; import java.util.ArrayList; import java.util.Arrays; @@ -45,6 +50,8 @@ public class RetrySettingsComposer { private static final Map STATIC_TYPES = createStaticTypes(); + private static final TypeNode STATUS_CODE_CODE_TYPE = + TypeNode.withReference(ConcreteReference.withClazz(StatusCode.Code.class)); public static BlockStatement createRetryParamDefinitionsBlock( Service service, @@ -116,6 +123,88 @@ public static BlockStatement createRetryParamDefinitionsBlock( .build(); } + public static BlockStatement createRetryCodesDefinitionsBlock( + Service service, + GapicServiceConfig serviceConfig, + VariableExpr retryCodesDefinitionsClassMemberVarExpr) { + TypeNode definitionsType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(ImmutableMap.Builder.class) + .setGenerics(retryCodesDefinitionsClassMemberVarExpr.type().reference().generics()) + .build()); + VariableExpr definitionsVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(definitionsType).setName("definitions").build()); + + List bodyExprs = new ArrayList<>(); + // Create the first expr. + bodyExprs.add( + AssignmentExpr.builder() + .setVariableExpr(definitionsVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr( + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("ImmutableMap")) + .setMethodName("builder") + .setReturnType(definitionsVarExpr.type()) + .build()) + .build()); + + for (Map.Entry> codeEntry : + serviceConfig.getAllRetryCodes(service).entrySet()) { + bodyExprs.add( + createRetryCodeDefinitionExpr( + codeEntry.getKey(), codeEntry.getValue(), definitionsVarExpr)); + } + + // Reassign the new codes. + bodyExprs.add( + AssignmentExpr.builder() + .setVariableExpr(retryCodesDefinitionsClassMemberVarExpr) + .setValueExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(definitionsVarExpr) + .setMethodName("build") + .setReturnType(retryCodesDefinitionsClassMemberVarExpr.type()) + .build()) + .build()); + + // Put everything together. + return BlockStatement.builder() + .setIsStatic(true) + .setBody( + bodyExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList())) + .build(); + } + + private static Expr createRetryCodeDefinitionExpr( + String codeName, List retryCodes, VariableExpr definitionsVarExpr) { + // Construct something like `definitions.put("code_name", + // ImmutableSet.copYOf(Lists.newArrayList()));` + MethodInvocationExpr codeListExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("Lists")) + .setGenerics(Arrays.asList(STATUS_CODE_CODE_TYPE.reference())) + .setMethodName("newArrayList") + .setArguments( + retryCodes.stream() + .map(c -> toStatusCodeEnumRefExpr(c)) + .collect(Collectors.toList())) + .build(); + + MethodInvocationExpr codeSetExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("ImmutableSet")) + .setMethodName("copyOf") + .setArguments(codeListExpr) + .build(); + return MethodInvocationExpr.builder() + .setExprReferenceExpr(definitionsVarExpr) + .setMethodName("put") + .setArguments(ValueExpr.withValue(StringObjectValue.withValue(codeName)), codeSetExpr) + .build(); + } + private static List createRetrySettingsExprs( String settingsName, GapicRetrySettings settings, @@ -240,9 +329,19 @@ private static List createRetrySettingsExprs( return Arrays.asList(settingsAssignExpr, definitionsPutExpr); } + private static EnumRefExpr toStatusCodeEnumRefExpr(Code code) { + return EnumRefExpr.builder().setType(STATUS_CODE_CODE_TYPE).setName(code.name()).build(); + } + private static Map createStaticTypes() { List concreteClazzes = - Arrays.asList(org.threeten.bp.Duration.class, ImmutableMap.class, RetrySettings.class); + Arrays.asList( + org.threeten.bp.Duration.class, + ImmutableMap.class, + ImmutableSet.class, + Lists.class, + RetrySettings.class, + StatusCode.class); return concreteClazzes.stream() .collect( Collectors.toMap( 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 8e35e40ef9..68fbdd1c1b 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 @@ -1121,7 +1121,7 @@ private static List createNestedClassStatements( .map(v -> varDeclFn.apply(v)) .collect(Collectors.toList())); - // Declare the RETRYABLE_CODE_DEFNITIONS field. + // Declare the RETRYABLE_CODE_DEFINITIONS field. Function varStaticDeclFn = v -> v.toBuilder() @@ -1135,8 +1135,13 @@ private static List createNestedClassStatements( List statements = new ArrayList<>(); statements.addAll( exprs.stream().map(e -> exprStatementFn.apply(e)).collect(Collectors.toList())); + + // Declare and set the RETRYABLE_CODE_DEFINITIONS field. statements.add( exprStatementFn.apply((varStaticDeclFn.apply(NESTED_RETRYABLE_CODE_DEFINITIONS_VAR_EXPR)))); + statements.add( + RetrySettingsComposer.createRetryCodesDefinitionsBlock( + service, serviceConfig, NESTED_RETRYABLE_CODE_DEFINITIONS_VAR_EXPR)); // Declare the RETRY_PARAM_DEFINITIONS field. statements.add( 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 eff9a8202a..0dca501d54 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 @@ -28,6 +28,7 @@ filegroup( ], test_class = "com.google.api.generator.gapic.composer.{0}".format(test_name), deps = [ + "//:rpc_java_proto", "//:service_config_java_proto", "//src/main/java/com/google/api/generator/engine/ast", "//src/main/java/com/google/api/generator/engine/writer", @@ -35,6 +36,7 @@ filegroup( "//src/main/java/com/google/api/generator/gapic/model", "//src/main/java/com/google/api/generator/gapic/protoparser", "//src/test/java/com/google/api/generator/gapic/testdata:showcase_java_proto", + "@com_google_api_gax_java//jar", "@com_google_protobuf//:protobuf_java", "@com_google_truth_truth//jar", "@junit_junit//jar", 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 eafc028566..03fef892ff 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 @@ -17,6 +17,7 @@ import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertTrue; +import com.google.api.gax.rpc.StatusCode; import com.google.api.generator.engine.ast.BlockStatement; import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.TypeNode; @@ -31,6 +32,7 @@ import com.google.api.generator.gapic.protoparser.Parser; import com.google.api.generator.gapic.protoparser.ServiceConfigParser; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; import com.google.showcase.v1beta1.EchoOuterClass; @@ -51,6 +53,8 @@ public class RetrySettingsComposerTest { "src/test/java/com/google/api/generator/gapic/testdata/"; private static final VariableExpr RETRY_PARAM_DEFINITIONS_VAR_EXPR = createRetryParamDefinitionsVarExpr(); + private static final VariableExpr RETRY_CODES_DEFINITIONS_VAR_EXPR = + createRetryableCodesDefinitionsVarExpr(); private JavaWriterVisitor writerVisitor; @@ -135,6 +139,101 @@ public void paramDefinitionsBlock_basic() { assertEquals(expected, writerVisitor.write()); } + @Test + public void codesDefinitionsBlock_noConfigsFound() { + FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor(); + ServiceDescriptor echoServiceDescriptor = echoFileDescriptor.getServices().get(0); + Map messageTypes = Parser.parseMessages(echoFileDescriptor); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + assertEquals(1, services.size()); + + Service service = services.get(0); + + String jsonFilename = "retrying_grpc_service_config.json"; + Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + assertTrue(serviceConfigOpt.isPresent()); + GapicServiceConfig serviceConfig = serviceConfigOpt.get(); + + BlockStatement paramDefinitionsBlock = + RetrySettingsComposer.createRetryCodesDefinitionsBlock( + service, serviceConfig, RETRY_CODES_DEFINITIONS_VAR_EXPR); + + paramDefinitionsBlock.accept(writerVisitor); + String expected = + createLines( + "static {\n", + "ImmutableMap.Builder> definitions =" + + " ImmutableMap.builder();\n", + "definitions.put(\"no_retry_codes\"," + + " ImmutableSet.copyOf(Lists.newArrayList()));\n", + "RETRYABLE_CODE_DEFINITIONS = definitions.build();\n", + "}\n"); + assertEquals(expected, writerVisitor.write()); + } + + @Test + public void codesDefinitionsBlock_basic() { + FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor(); + ServiceDescriptor echoServiceDescriptor = echoFileDescriptor.getServices().get(0); + Map messageTypes = Parser.parseMessages(echoFileDescriptor); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + assertEquals(1, services.size()); + + Service service = services.get(0); + + String jsonFilename = "showcase_grpc_service_config.json"; + Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + assertTrue(serviceConfigOpt.isPresent()); + GapicServiceConfig serviceConfig = serviceConfigOpt.get(); + + BlockStatement paramDefinitionsBlock = + RetrySettingsComposer.createRetryCodesDefinitionsBlock( + service, serviceConfig, RETRY_CODES_DEFINITIONS_VAR_EXPR); + + paramDefinitionsBlock.accept(writerVisitor); + String expected = + createLines( + "static {\n", + "ImmutableMap.Builder> definitions =" + + " ImmutableMap.builder();\n", + "definitions.put(\"retry_policy_1_codes\"," + + " ImmutableSet.copyOf(Lists.newArrayList(StatusCode.Code.UNAVAILABLE," + + " StatusCode.Code.UNKNOWN)));\n", + "definitions.put(\"no_retry_0_codes\"," + + " ImmutableSet.copyOf(Lists.newArrayList()));\n", + "RETRYABLE_CODE_DEFINITIONS = definitions.build();\n", + "}\n"); + assertEquals(expected, writerVisitor.write()); + } + + private static VariableExpr createRetryableCodesDefinitionsVarExpr() { + TypeNode immutableSetType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(ImmutableSet.class) + .setGenerics(Arrays.asList(ConcreteReference.withClazz(StatusCode.Code.class))) + .build()); + TypeNode varType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(ImmutableMap.class) + .setGenerics( + Arrays.asList(TypeNode.STRING, immutableSetType).stream() + .map(t -> t.reference()) + .collect(Collectors.toList())) + .build()); + return VariableExpr.withVariable( + Variable.builder().setType(varType).setName("RETRYABLE_CODE_DEFINITIONS").build()); + } + private static VariableExpr createRetryParamDefinitionsVarExpr() { TypeNode retrySettingsType = TypeNode.withReference( 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 ced1b897c7..bb73d72816 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 @@ -109,6 +109,7 @@ public void generateServiceClasses() { + "import com.google.common.collect.ImmutableList;\n" + "import com.google.common.collect.ImmutableMap;\n" + "import com.google.common.collect.ImmutableSet;\n" + + "import com.google.common.collect.Lists;\n" + "import com.google.longrunning.Operation;\n" + "import com.google.showcase.v1beta1.BlockRequest;\n" + "import com.google.showcase.v1beta1.BlockResponse;\n" @@ -352,6 +353,21 @@ public void generateServiceClasses() { + " blockSettings;\n" + " private static final ImmutableMap>\n" + " RETRYABLE_CODE_DEFINITIONS;\n" + + "\n" + + " static {\n" + + " ImmutableMap.Builder> definitions =\n" + + " ImmutableMap.builder();\n" + + " definitions.put(\n" + + " \"retry_policy_1_codes\",\n" + + " ImmutableSet.copyOf(\n" + + " Lists.newArrayList(\n" + + " StatusCode.Code.UNAVAILABLE, StatusCode.Code.UNKNOWN)));\n" + + " definitions.put(\n" + + " \"no_retry_0_codes\"," + + " ImmutableSet.copyOf(Lists.newArrayList()));\n" + + " RETRYABLE_CODE_DEFINITIONS = definitions.build();\n" + + " }\n" + + "\n" + " private static final ImmutableMap" + " RETRY_PARAM_DEFINITIONS;\n" + "\n" From 1c54ec10693ce97676ecf1af917acd1991351d6d Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 27 Aug 2020 12:27:18 -0700 Subject: [PATCH 15/24] feat: add initDefaults() to ServiceStubSettings codegen --- .../gapic/composer/RetrySettingsComposer.java | 54 +++++++++ .../ServiceStubSettingsClassComposer.java | 48 +++++++- .../composer/RetrySettingsComposerTest.java | 104 ++++++++++++++++++ .../ServiceStubSettingsClassComposerTest.java | 30 +++++ 4 files changed, 233 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java index 612945c8ca..2519532fb1 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java @@ -32,7 +32,9 @@ import com.google.api.generator.engine.ast.VariableExpr; import com.google.api.generator.gapic.model.GapicRetrySettings; import com.google.api.generator.gapic.model.GapicServiceConfig; +import com.google.api.generator.gapic.model.Method; import com.google.api.generator.gapic.model.Service; +import com.google.api.generator.gapic.utils.JavaStyle; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -53,6 +55,12 @@ public class RetrySettingsComposer { private static final TypeNode STATUS_CODE_CODE_TYPE = TypeNode.withReference(ConcreteReference.withClazz(StatusCode.Code.class)); + // TODO(miraleung): Determine defaults here. + private static final long LRO_DEFAULT_INITIAL_POLL_DELAY_MILLIS = 9999; + private static final long LRO_DEFAULT_POLL_DELAY_MULTIPLIER = 1; + private static final long LRO_DEFAULT_MAX_POLL_DELAY_MILLIS = 8888; + private static final long LRO_DEFAULT_TOTAL_POLL_TIMEOUT_MILLIS = 7777; + public static BlockStatement createRetryParamDefinitionsBlock( Service service, GapicServiceConfig serviceConfig, @@ -177,6 +185,52 @@ public static BlockStatement createRetryCodesDefinitionsBlock( .build(); } + public static Expr createSimpleBuilderSettingsExpr( + Service service, + GapicServiceConfig serviceConfig, + Method method, + VariableExpr builderVarExpr, + VariableExpr retryableCodeDefsVarExpr, + VariableExpr retryParamDefsVarExpr) { + String codeName = serviceConfig.getRetryCodeName(service, method); + String retryParamName = serviceConfig.getRetryParamsName(service, method); + String settingsGetterMethodName = + String.format("%sSettings", JavaStyle.toLowerCamelCase(method.name())); + + Expr builderSettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(builderVarExpr) + .setMethodName(settingsGetterMethodName) + .build(); + + Function strValExprFn = + s -> ValueExpr.withValue(StringObjectValue.withValue(s)); + builderSettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(builderSettingsExpr) + .setMethodName("setRetryableCodes") + .setArguments( + MethodInvocationExpr.builder() + .setExprReferenceExpr(retryableCodeDefsVarExpr) + .setMethodName("get") + .setArguments(strValExprFn.apply(codeName)) + .build()) + .build(); + builderSettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(builderSettingsExpr) + .setMethodName("setRetrySettings") + .setArguments( + MethodInvocationExpr.builder() + .setExprReferenceExpr(retryParamDefsVarExpr) + .setMethodName("get") + .setArguments(strValExprFn.apply(retryParamName)) + .build()) + .build(); + + return builderSettingsExpr; + } + private static Expr createRetryCodeDefinitionExpr( String codeName, List retryCodes, VariableExpr definitionsVarExpr) { // Construct something like `definitions.put("code_name", 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 68fbdd1c1b..eceb426fe2 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 @@ -1100,7 +1100,9 @@ private static ClassDefinition createNestedBuilderClass( .setExtendsType(extendsType) .setStatements( createNestedClassStatements(service, serviceConfig, nestedMethodSettingsMemberVarExprs)) - .setMethods(createNestedClassMethods(nestedMethodSettingsMemberVarExprs, types)) + .setMethods( + createNestedClassMethods( + service, serviceConfig, nestedMethodSettingsMemberVarExprs, types)) .build(); } @@ -1155,15 +1157,55 @@ private static List createNestedClassStatements( } private static List createNestedClassMethods( - Map nestedMethodSettingsMemberVarExprs, Map types) { + Service service, + GapicServiceConfig serviceConfig, + Map nestedMethodSettingsMemberVarExprs, + Map types) { List nestedClassMethods = new ArrayList<>(); nestedClassMethods.addAll( createNestedClassConstructorMethods(nestedMethodSettingsMemberVarExprs, types)); + nestedClassMethods.add(createNestedClassInitDefaultsMethod(service, serviceConfig, types)); - // TODO(miraleung): initDefaults(). + // TODO(miraleung): More methods. return nestedClassMethods; } + private static MethodDefinition createNestedClassInitDefaultsMethod( + Service service, GapicServiceConfig serviceConfig, Map types) { + TypeNode builderType = types.get(NESTED_BUILDER_CLASS_NAME); + VariableExpr builderVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(builderType).setName("builder").build()); + + List bodyExprs = new ArrayList<>(); + // Iterate through methods twice to so we can have LRO expressions appear last. + for (Method method : service.methods()) { + Method.Stream streamKind = method.stream(); + if (streamKind.equals(Method.Stream.CLIENT) || streamKind.equals(Method.Stream.BIDI)) { + continue; + } + bodyExprs.add( + RetrySettingsComposer.createSimpleBuilderSettingsExpr( + service, + serviceConfig, + method, + builderVarExpr, + NESTED_RETRYABLE_CODE_DEFINITIONS_VAR_EXPR, + NESTED_RETRY_PARAM_DEFINITIONS_VAR_EXPR)); + } + + return MethodDefinition.builder() + .setScope(ScopeNode.PRIVATE) + .setIsStatic(true) + .setReturnType(builderType) + .setName("initDefaults") + .setArguments(builderVarExpr.toBuilder().setIsDecl(true).build()) + .setBody( + bodyExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList())) + .setReturnExpr(builderVarExpr) + .build(); + } + private static List createNestedClassConstructorMethods( Map nestedMethodSettingsMemberVarExprs, Map types) { TypeNode builderType = types.get(NESTED_BUILDER_CLASS_NAME); 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 03fef892ff..3152d2a0ff 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 @@ -14,12 +14,14 @@ package com.google.api.generator.gapic.composer; +import static com.google.common.truth.Truth.assertThat; import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertTrue; import com.google.api.gax.rpc.StatusCode; import com.google.api.generator.engine.ast.BlockStatement; import com.google.api.generator.engine.ast.ConcreteReference; +import com.google.api.generator.engine.ast.Expr; import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.VaporReference; import com.google.api.generator.engine.ast.Variable; @@ -27,6 +29,7 @@ import com.google.api.generator.engine.writer.JavaWriterVisitor; 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.Parser; @@ -214,6 +217,107 @@ public void codesDefinitionsBlock_basic() { assertEquals(expected, writerVisitor.write()); } + @Test + public void simplerBuilderExpr_basic() { + FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor(); + ServiceDescriptor echoServiceDescriptor = echoFileDescriptor.getServices().get(0); + Map messageTypes = Parser.parseMessages(echoFileDescriptor); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + assertEquals(1, services.size()); + + Service service = services.get(0); + + String jsonFilename = "showcase_grpc_service_config.json"; + Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + assertTrue(serviceConfigOpt.isPresent()); + GapicServiceConfig serviceConfig = serviceConfigOpt.get(); + + // Regular method. + Method echoMethod = findMethod(service, "Echo"); + assertThat(echoMethod).isNotNull(); + + VariableExpr builderVarExpr = createBuilderVarExpr(service); + Expr builderExpr = + RetrySettingsComposer.createSimpleBuilderSettingsExpr( + service, + serviceConfig, + echoMethod, + builderVarExpr, + RETRY_CODES_DEFINITIONS_VAR_EXPR, + RETRY_PARAM_DEFINITIONS_VAR_EXPR); + builderExpr.accept(writerVisitor); + String expected = + createLines( + "builder.echoSettings()" + + ".setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get(\"retry_policy_1_codes\"))" + + ".setRetrySettings(RETRY_PARAM_DEFINITIONS.get(\"retry_policy_1_params\"))"); + assertEquals(expected, writerVisitor.write()); + + // Server-streaming method. + Method expandMethod = findMethod(service, "Expand"); + assertThat(expandMethod).isNotNull(); + builderExpr = + RetrySettingsComposer.createSimpleBuilderSettingsExpr( + service, + serviceConfig, + expandMethod, + builderVarExpr, + RETRY_CODES_DEFINITIONS_VAR_EXPR, + RETRY_PARAM_DEFINITIONS_VAR_EXPR); + writerVisitor.clear(); + builderExpr.accept(writerVisitor); + expected = + createLines( + "builder.expandSettings()" + + ".setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get(\"retry_policy_1_codes\"))" + + ".setRetrySettings(RETRY_PARAM_DEFINITIONS.get(\"retry_policy_1_params\"))"); + assertEquals(expected, writerVisitor.write()); + + // LRO method. + Method waitMethod = findMethod(service, "Wait"); + assertThat(waitMethod).isNotNull(); + builderExpr = + RetrySettingsComposer.createSimpleBuilderSettingsExpr( + service, + serviceConfig, + waitMethod, + builderVarExpr, + RETRY_CODES_DEFINITIONS_VAR_EXPR, + RETRY_PARAM_DEFINITIONS_VAR_EXPR); + writerVisitor.clear(); + builderExpr.accept(writerVisitor); + expected = + createLines( + "builder.waitSettings()" + + ".setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get(\"no_retry_0_codes\"))" + + ".setRetrySettings(RETRY_PARAM_DEFINITIONS.get(\"no_retry_0_params\"))"); + 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 VariableExpr createBuilderVarExpr(Service service) { + TypeNode builderType = + TypeNode.withReference( + VaporReference.builder() + .setPakkage(String.format("%s.stub", service.pakkage())) + .setName("Builder") + .build()); + return VariableExpr.withVariable( + Variable.builder().setType(builderType).setName("builder").build()); + } + private static VariableExpr createRetryableCodesDefinitionsVarExpr() { TypeNode immutableSetType = TypeNode.withReference( 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 bb73d72816..661d681349 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 @@ -418,6 +418,36 @@ public void generateServiceClasses() { + " echoSettings, pagedExpandSettings, waitSettings, blockSettings);\n" + " initDefaults(this);\n" + " }\n" + + "\n" + + " private static Builder initDefaults(Builder builder) {\n" + + " builder\n" + + " .echoSettings()\n" + + " " + + " .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get(\"retry_policy_1_codes\"))\n" + + " " + + " .setRetrySettings(RETRY_PARAM_DEFINITIONS.get(\"retry_policy_1_params\"));\n" + + " builder\n" + + " .expandSettings()\n" + + " " + + " .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get(\"retry_policy_1_codes\"))\n" + + " " + + " .setRetrySettings(RETRY_PARAM_DEFINITIONS.get(\"retry_policy_1_params\"));\n" + + " builder\n" + + " .pagedExpandSettings()\n" + + " " + + " .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get(\"retry_policy_1_codes\"))\n" + + " " + + " .setRetrySettings(RETRY_PARAM_DEFINITIONS.get(\"retry_policy_1_params\"));\n" + + " builder\n" + + " .waitSettings()\n" + + " .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get(\"no_retry_0_codes\"))\n" + + " .setRetrySettings(RETRY_PARAM_DEFINITIONS.get(\"no_retry_0_params\"));\n" + + " builder\n" + + " .blockSettings()\n" + + " .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get(\"no_retry_0_codes\"))\n" + + " .setRetrySettings(RETRY_PARAM_DEFINITIONS.get(\"no_retry_0_params\"));\n" + + " return builder;\n" + + " }\n" + " }\n" + "}\n"; } From c50a204dc8dff6b2144549b4c94dea127d58e0f1 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 27 Aug 2020 13:49:39 -0700 Subject: [PATCH 16/24] feat: add LRO to ServiceStubSettings.Builder.initDefaults --- .../gapic/composer/RetrySettingsComposer.java | 292 +++++++++++++++--- .../ServiceStubSettingsClassComposer.java | 13 + .../composer/RetrySettingsComposerTest.java | 54 +++- .../ServiceStubSettingsClassComposerTest.java | 30 ++ 4 files changed, 349 insertions(+), 40 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java index 2519532fb1..22608b9f0d 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java @@ -14,8 +14,12 @@ package com.google.api.generator.gapic.composer; +import com.google.api.gax.grpc.ProtoOperationTransformers; +import com.google.api.gax.longrunning.OperationSnapshot; +import com.google.api.gax.longrunning.OperationTimedPollAlgorithm; import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.rpc.StatusCode; +import com.google.api.gax.rpc.UnaryCallSettings; import com.google.api.generator.engine.ast.AssignmentExpr; import com.google.api.generator.engine.ast.BlockStatement; import com.google.api.generator.engine.ast.ConcreteReference; @@ -56,10 +60,12 @@ public class RetrySettingsComposer { TypeNode.withReference(ConcreteReference.withClazz(StatusCode.Code.class)); // TODO(miraleung): Determine defaults here. - private static final long LRO_DEFAULT_INITIAL_POLL_DELAY_MILLIS = 9999; - private static final long LRO_DEFAULT_POLL_DELAY_MULTIPLIER = 1; - private static final long LRO_DEFAULT_MAX_POLL_DELAY_MILLIS = 8888; - private static final long LRO_DEFAULT_TOTAL_POLL_TIMEOUT_MILLIS = 7777; + // Default values for LongRunningConfig fields. + private static final long LRO_DEFAULT_INITIAL_POLL_DELAY_MILLIS = 500; + private static final double LRO_DEFAULT_POLL_DELAY_MULTIPLIER = 1.5; + private static final long LRO_DEFAULT_MAX_POLL_DELAY_MILLIS = 5000; + private static final long LRO_DEFAULT_TOTAL_POLL_TIMEOUT_MILLS = 300000; + private static final double LRO_DEFAULT_MAX_RPC_TIMEOUT = 1.0; public static BlockStatement createRetryParamDefinitionsBlock( Service service, @@ -231,6 +237,134 @@ public static Expr createSimpleBuilderSettingsExpr( return builderSettingsExpr; } + public static Expr createLroSettingsBuilderExpr( + Service service, + GapicServiceConfig serviceConfig, + Method method, + VariableExpr builderVarExpr, + VariableExpr retryableCodeDefsVarExpr, + VariableExpr retryParamDefsVarExpr) { + Preconditions.checkState( + method.hasLro(), + String.format( + "Tried to create LRO settings initialization for non-LRO method %s", method.name())); + + String codeName = serviceConfig.getRetryCodeName(service, method); + String retryParamName = serviceConfig.getRetryParamsName(service, method); + String settingsGetterMethodName = + String.format("%sOperationSettings", JavaStyle.toLowerCamelCase(method.name())); + + Function strValExprFn = + s -> ValueExpr.withValue(StringObjectValue.withValue(s)); + + // Argument for setInitialCallSettings. + Expr unaryCallSettingsExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("UnaryCallSettings")) + .setGenerics( + Arrays.asList( + method.inputType().reference(), + STATIC_TYPES.get("OperationSnapshot").reference())) + .setMethodName("newUnaryCallSettingsBuilder") + .build(); + unaryCallSettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(unaryCallSettingsExpr) + .setMethodName("setRetryableCodes") + .setArguments( + MethodInvocationExpr.builder() + .setExprReferenceExpr(retryableCodeDefsVarExpr) + .setMethodName("get") + .setArguments(strValExprFn.apply(codeName)) + .build()) + .build(); + unaryCallSettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(unaryCallSettingsExpr) + .setMethodName("setRetrySettings") + .setArguments( + MethodInvocationExpr.builder() + .setExprReferenceExpr(retryParamDefsVarExpr) + .setMethodName("get") + .setArguments(strValExprFn.apply(retryParamName)) + .build()) + .build(); + unaryCallSettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(unaryCallSettingsExpr) + .setMethodName("build") + .build(); + + Expr builderSettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(builderVarExpr) + .setMethodName(settingsGetterMethodName) + .build(); + builderSettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(builderSettingsExpr) + .setMethodName("setInitialCallSettings") + .setArguments(unaryCallSettingsExpr) + .build(); + + Function classFieldRefFn = + t -> + VariableExpr.builder() + .setVariable( + Variable.builder() + .setType(TypeNode.withReference(ConcreteReference.withClazz(Class.class))) + .setName("class") + .build()) + .setStaticReferenceType(t) + .build(); + builderSettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(builderSettingsExpr) + .setMethodName("setResponseTransformer") + .setArguments( + MethodInvocationExpr.builder() + .setStaticReferenceType( + TypeNode.withReference( + ConcreteReference.withClazz( + ProtoOperationTransformers.ResponseTransformer.class))) + .setMethodName("create") + .setArguments(classFieldRefFn.apply(method.lro().responseType())) + .build()) + .build(); + builderSettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(builderSettingsExpr) + .setMethodName("setMetadataTransformer") + .setArguments( + MethodInvocationExpr.builder() + .setStaticReferenceType( + TypeNode.withReference( + ConcreteReference.withClazz( + ProtoOperationTransformers.MetadataTransformer.class))) + .setMethodName("create") + .setArguments(classFieldRefFn.apply(method.lro().metadataType())) + .build()) + .build(); + + // TODO(miraleung): Determine fianl LRO settings values here. + Expr lroRetrySettingsExpr = createLroRetrySettingsExpr(); + Expr pollAlgoExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("OperationTimedPollAlgorithm")) + .setMethodName("create") + .setArguments(lroRetrySettingsExpr) + .build(); + + builderSettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(builderSettingsExpr) + .setMethodName("setPollingAlgorithm") + .setArguments(pollAlgoExpr) + .build(); + + return builderSettingsExpr; + } + private static Expr createRetryCodeDefinitionExpr( String codeName, List retryCodes, VariableExpr definitionsVarExpr) { // Construct something like `definitions.put("code_name", @@ -264,28 +398,6 @@ private static List createRetrySettingsExprs( GapicRetrySettings settings, VariableExpr settingsVarExpr, VariableExpr definitionsVarExpr) { - Function durationToMillisValExprFn = - d -> - ValueExpr.withValue( - PrimitiveValue.builder() - .setType(TypeNode.LONG) - .setValue(String.format("%dL", Durations.toMillis(d))) - .build()); - Function floatToValExprFn = - f -> - ValueExpr.withValue( - PrimitiveValue.builder() - .setType(TypeNode.DOUBLE) - .setValue(String.format("%.1f", f)) - .build()); - Function durationMillisMethodFn = - v -> - MethodInvocationExpr.builder() - .setStaticReferenceType(STATIC_TYPES.get("Duration")) - .setMethodName("ofMillis") - .setArguments(v) - .build(); - Expr settingsBuilderExpr = MethodInvocationExpr.builder() .setStaticReferenceType(STATIC_TYPES.get("RetrySettings")) @@ -301,16 +413,14 @@ private static List createRetrySettingsExprs( MethodInvocationExpr.builder() .setExprReferenceExpr(settingsBuilderExpr) .setMethodName("setInitialRetryDelay") - .setArguments( - durationMillisMethodFn.apply( - durationToMillisValExprFn.apply(retryPolicy.getInitialBackoff()))) + .setArguments(createDurationOfMillisExpr(toValExpr(retryPolicy.getInitialBackoff()))) .build(); settingsBuilderExpr = MethodInvocationExpr.builder() .setExprReferenceExpr(settingsBuilderExpr) .setMethodName("setRetryDelayMultiplier") - .setArguments(floatToValExprFn.apply(retryPolicy.getBackoffMultiplier())) + .setArguments(toValExpr(retryPolicy.getBackoffMultiplier())) .build(); Preconditions.checkState( @@ -320,9 +430,7 @@ private static List createRetrySettingsExprs( MethodInvocationExpr.builder() .setExprReferenceExpr(settingsBuilderExpr) .setMethodName("setMaxRetryDelay") - .setArguments( - durationMillisMethodFn.apply( - durationToMillisValExprFn.apply(retryPolicy.getMaxBackoff()))) + .setArguments(createDurationOfMillisExpr(toValExpr(retryPolicy.getMaxBackoff()))) .build(); } @@ -331,8 +439,7 @@ private static List createRetrySettingsExprs( MethodInvocationExpr.builder() .setExprReferenceExpr(settingsBuilderExpr) .setMethodName("setInitialRpcTimeout") - .setArguments( - durationMillisMethodFn.apply(durationToMillisValExprFn.apply(settings.timeout()))) + .setArguments(createDurationOfMillisExpr(toValExpr(settings.timeout()))) .build(); } @@ -352,9 +459,7 @@ private static List createRetrySettingsExprs( MethodInvocationExpr.builder() .setExprReferenceExpr(settingsBuilderExpr) .setMethodName(setterMethodName) - .setArguments( - durationMillisMethodFn.apply( - durationToMillisValExprFn.apply(settings.timeout()))) + .setArguments(createDurationOfMillisExpr(toValExpr(settings.timeout()))) .build(); } } @@ -383,10 +488,115 @@ private static List createRetrySettingsExprs( return Arrays.asList(settingsAssignExpr, definitionsPutExpr); } + private static Expr createLroRetrySettingsExpr() { + // TODO(miraleung): Determine fianl LRO settings values here. + Expr lroRetrySettingsExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("RetrySettings")) + .setMethodName("newBuilder") + .build(); + + lroRetrySettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(lroRetrySettingsExpr) + .setMethodName("setInitialRetryDelay") + .setArguments( + createDurationOfMillisExpr(toValExpr(LRO_DEFAULT_INITIAL_POLL_DELAY_MILLIS))) + .build(); + + lroRetrySettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(lroRetrySettingsExpr) + .setMethodName("setRetryDelayMultiplier") + .setArguments(toValExpr(LRO_DEFAULT_POLL_DELAY_MULTIPLIER)) + .build(); + + lroRetrySettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(lroRetrySettingsExpr) + .setMethodName("setMaxRetryDelay") + .setArguments(createDurationOfMillisExpr(toValExpr(LRO_DEFAULT_MAX_POLL_DELAY_MILLIS))) + .build(); + + Expr zeroDurationExpr = + EnumRefExpr.builder().setType(STATIC_TYPES.get("Duration")).setName("ZERO").build(); + // TODO(miraleung): Find a way to add an "// ignored" comment here. + lroRetrySettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(lroRetrySettingsExpr) + .setMethodName("setInitialRpcTimeout") + .setArguments(zeroDurationExpr) + .build(); + + // TODO(miraleung): Find a way to add an "// ignored" comment here. + lroRetrySettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(lroRetrySettingsExpr) + .setMethodName("setRpcTimeoutMultiplier") + .setArguments(toValExpr(LRO_DEFAULT_MAX_RPC_TIMEOUT)) + .build(); + + // TODO(miraleung): Find a way to add an "// ignored" comment here. + lroRetrySettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(lroRetrySettingsExpr) + .setMethodName("setMaxRpcTimeout") + .setArguments(zeroDurationExpr) + .build(); + + lroRetrySettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(lroRetrySettingsExpr) + .setMethodName("setTotalTimeout") + .setArguments( + createDurationOfMillisExpr(toValExpr(LRO_DEFAULT_TOTAL_POLL_TIMEOUT_MILLS))) + .build(); + + lroRetrySettingsExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(lroRetrySettingsExpr) + .setMethodName("build") + .build(); + + return lroRetrySettingsExpr; + } + private static EnumRefExpr toStatusCodeEnumRefExpr(Code code) { return EnumRefExpr.builder().setType(STATUS_CODE_CODE_TYPE).setName(code.name()).build(); } + private static ValueExpr toValExpr(long longValue) { + return ValueExpr.withValue( + PrimitiveValue.builder() + .setType(TypeNode.LONG) + .setValue(String.format("%dL", longValue)) + .build()); + } + + private static ValueExpr toValExpr(float floatValue) { + return toValExpr((double) floatValue); + } + + private static ValueExpr toValExpr(double val) { + return ValueExpr.withValue( + PrimitiveValue.builder() + .setType(TypeNode.DOUBLE) + .setValue(String.format("%.1f", val)) + .build()); + } + + private static ValueExpr toValExpr(Duration duration) { + return toValExpr(Durations.toMillis(duration)); + } + + private static MethodInvocationExpr createDurationOfMillisExpr(ValueExpr valExpr) { + return MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("Duration")) + .setMethodName("ofMillis") + .setArguments(valExpr) + .build(); + } + private static Map createStaticTypes() { List concreteClazzes = Arrays.asList( @@ -394,8 +604,12 @@ private static Map createStaticTypes() { ImmutableMap.class, ImmutableSet.class, Lists.class, + OperationSnapshot.class, + OperationTimedPollAlgorithm.class, + ProtoOperationTransformers.class, RetrySettings.class, - StatusCode.class); + StatusCode.class, + UnaryCallSettings.class); return concreteClazzes.stream() .collect( Collectors.toMap( 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 eceb426fe2..a511558462 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 @@ -1193,6 +1193,19 @@ private static MethodDefinition createNestedClassInitDefaultsMethod( NESTED_RETRYABLE_CODE_DEFINITIONS_VAR_EXPR, NESTED_RETRY_PARAM_DEFINITIONS_VAR_EXPR)); } + for (Method method : service.methods()) { + if (!method.hasLro()) { + continue; + } + bodyExprs.add( + RetrySettingsComposer.createLroSettingsBuilderExpr( + service, + serviceConfig, + method, + builderVarExpr, + NESTED_RETRYABLE_CODE_DEFINITIONS_VAR_EXPR, + NESTED_RETRY_PARAM_DEFINITIONS_VAR_EXPR)); + } return MethodDefinition.builder() .setScope(ScopeNode.PRIVATE) 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 3152d2a0ff..d4aae0abb3 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 @@ -218,7 +218,7 @@ public void codesDefinitionsBlock_basic() { } @Test - public void simplerBuilderExpr_basic() { + public void simpleBuilderExpr_basic() { FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor(); ServiceDescriptor echoServiceDescriptor = echoFileDescriptor.getServices().get(0); Map messageTypes = Parser.parseMessages(echoFileDescriptor); @@ -298,6 +298,58 @@ public void simplerBuilderExpr_basic() { assertEquals(expected, writerVisitor.write()); } + @Test + public void lroBuilderExpr() { + FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor(); + ServiceDescriptor echoServiceDescriptor = echoFileDescriptor.getServices().get(0); + Map messageTypes = Parser.parseMessages(echoFileDescriptor); + Map resourceNames = Parser.parseResourceNames(echoFileDescriptor); + Set outputResourceNames = new HashSet<>(); + List services = + Parser.parseService(echoFileDescriptor, messageTypes, resourceNames, outputResourceNames); + assertEquals(1, services.size()); + + Service service = services.get(0); + + String jsonFilename = "showcase_grpc_service_config.json"; + Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + assertTrue(serviceConfigOpt.isPresent()); + GapicServiceConfig serviceConfig = serviceConfigOpt.get(); + + // LRO method. + Method waitMethod = findMethod(service, "Wait"); + assertThat(waitMethod).isNotNull(); + + VariableExpr builderVarExpr = createBuilderVarExpr(service); + Expr builderExpr = + RetrySettingsComposer.createLroSettingsBuilderExpr( + service, + serviceConfig, + waitMethod, + builderVarExpr, + RETRY_CODES_DEFINITIONS_VAR_EXPR, + RETRY_PARAM_DEFINITIONS_VAR_EXPR); + builderExpr.accept(writerVisitor); + String expected = + createLines( + "builder.waitOperationSettings()" + + ".setInitialCallSettings(UnaryCallSettings.newUnaryCallSettingsBuilder()" + + ".setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get(\"no_retry_0_codes\"))" + + ".setRetrySettings(RETRY_PARAM_DEFINITIONS.get(\"no_retry_0_params\")).build())" + + ".setResponseTransformer(ProtoOperationTransformers.ResponseTransformer.create(" + + "WaitResponse.class))" + + ".setMetadataTransformer(ProtoOperationTransformers.MetadataTransformer.create(" + + "WaitMetadata.class)).setPollingAlgorithm(OperationTimedPollAlgorithm.create(" + + "RetrySettings.newBuilder().setInitialRetryDelay(Duration.ofMillis(500L))" + + ".setRetryDelayMultiplier(1.5).setMaxRetryDelay(Duration.ofMillis(5000L))" + + ".setInitialRpcTimeout(Duration.ZERO).setRpcTimeoutMultiplier(1.0)" + + ".setMaxRpcTimeout(Duration.ZERO).setTotalTimeout(Duration.ofMillis(300000L))" + + ".build()))"); + assertEquals(expected, writerVisitor.write()); + } + private static Method findMethod(Service service, String methodName) { for (Method m : service.methods()) { if (m.name().equals(methodName)) { 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 661d681349..9c6c76eae8 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 @@ -90,6 +90,9 @@ public void generateServiceClasses() { + "import com.google.api.gax.grpc.GaxGrpcProperties;\n" + "import com.google.api.gax.grpc.GrpcTransportChannel;\n" + "import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;\n" + + "import com.google.api.gax.grpc.ProtoOperationTransformers;\n" + + "import com.google.api.gax.longrunning.OperationSnapshot;\n" + + "import com.google.api.gax.longrunning.OperationTimedPollAlgorithm;\n" + "import com.google.api.gax.retrying.RetrySettings;\n" + "import com.google.api.gax.rpc.ApiCallContext;\n" + "import com.google.api.gax.rpc.ApiClientHeaderProvider;\n" @@ -446,6 +449,33 @@ public void generateServiceClasses() { + " .blockSettings()\n" + " .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get(\"no_retry_0_codes\"))\n" + " .setRetrySettings(RETRY_PARAM_DEFINITIONS.get(\"no_retry_0_params\"));\n" + + " builder\n" + + " .waitOperationSettings()\n" + + " .setInitialCallSettings(\n" + + " UnaryCallSettings.newUnaryCallSettingsBuilder()\n" + + " " + + " .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get(\"no_retry_0_codes\"))\n" + + " " + + " .setRetrySettings(RETRY_PARAM_DEFINITIONS.get(\"no_retry_0_params\"))\n" + + " .build())\n" + + " .setResponseTransformer(\n" + + " " + + " ProtoOperationTransformers.ResponseTransformer.create(WaitResponse.class))\n" + + " .setMetadataTransformer(\n" + + " " + + " ProtoOperationTransformers.MetadataTransformer.create(WaitMetadata.class))\n" + + " .setPollingAlgorithm(\n" + + " OperationTimedPollAlgorithm.create(\n" + + " RetrySettings.newBuilder()\n" + + " .setInitialRetryDelay(Duration.ofMillis(500L))\n" + + " .setRetryDelayMultiplier(1.5)\n" + + " .setMaxRetryDelay(Duration.ofMillis(5000L))\n" + + " .setInitialRpcTimeout(Duration.ZERO)\n" + + " .setRpcTimeoutMultiplier(1.0)\n" + + " .setMaxRpcTimeout(Duration.ZERO)\n" + + " .setTotalTimeout(Duration.ofMillis(300000L))\n" + + " .build()));\n" + " return builder;\n" + " }\n" + " }\n" From 106528fc8395e02abe67f2d6812f49da2b59ae8c Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 27 Aug 2020 14:07:12 -0700 Subject: [PATCH 17/24] feat: add third ServiceStubSettings.Builder(settings) ctor --- .../ServiceStubSettingsClassComposer.java | 55 +++++++++++++++++-- .../ServiceStubSettingsClassComposerTest.java | 16 ++++++ 2 files changed, 67 insertions(+), 4 deletions(-) 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 a511558462..42127311a3 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 @@ -1163,7 +1163,7 @@ private static List createNestedClassMethods( Map types) { List nestedClassMethods = new ArrayList<>(); nestedClassMethods.addAll( - createNestedClassConstructorMethods(nestedMethodSettingsMemberVarExprs, types)); + createNestedClassConstructorMethods(service, nestedMethodSettingsMemberVarExprs, types)); nestedClassMethods.add(createNestedClassInitDefaultsMethod(service, serviceConfig, types)); // TODO(miraleung): More methods. @@ -1220,7 +1220,9 @@ private static MethodDefinition createNestedClassInitDefaultsMethod( } private static List createNestedClassConstructorMethods( - Map nestedMethodSettingsMemberVarExprs, Map types) { + Service service, + Map nestedMethodSettingsMemberVarExprs, + Map types) { TypeNode builderType = types.get(NESTED_BUILDER_CLASS_NAME); List ctorMethods = new ArrayList<>(); @@ -1324,7 +1326,7 @@ private static List createNestedClassConstructorMethods( }) .collect(Collectors.toList())); - ctorBodyExprs.add( + Expr unaryMethodSettingsBuildersAssignExpr = AssignmentExpr.builder() .setVariableExpr(NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR) .setValueExpr( @@ -1345,7 +1347,8 @@ private static List createNestedClassConstructorMethods( .collect(Collectors.toList())) .setReturnType(NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR.type()) .build()) - .build()); + .build(); + ctorBodyExprs.add(unaryMethodSettingsBuildersAssignExpr); ctorBodyExprs.add( MethodInvocationExpr.builder() @@ -1364,6 +1367,50 @@ private static List createNestedClassConstructorMethods( .collect(Collectors.toList())) .build()); + // Third constructor that takes a ServivceStubSettings. + TypeNode outerSettingsType = types.get(getThisClassName(service.name())); + VariableExpr settingsVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(outerSettingsType).setName("settings").build()); + ctorBodyExprs = new ArrayList<>(); + ctorBodyExprs.add( + ReferenceConstructorExpr.superBuilder() + .setType(builderType) + .setArguments(settingsVarExpr) + .build()); + // TODO(cleanup): Technically this should actually use the outer class's Settings + // members to avoid decoupling variable names. + ctorBodyExprs.addAll( + nestedMethodSettingsMemberVarExprs.values().stream() + .map( + v -> + AssignmentExpr.builder() + .setVariableExpr(v) + .setValueExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr( + VariableExpr.builder() + .setExprReferenceExpr(settingsVarExpr) + .setVariable(v.variable()) + .build()) + .setMethodName("toBuilder") + .setReturnType(v.type()) + .build()) + .build()) + .collect(Collectors.toList())); + ctorBodyExprs.add(unaryMethodSettingsBuildersAssignExpr); + + ctorMethods.add( + MethodDefinition.constructorBuilder() + .setScope(ScopeNode.PROTECTED) + .setReturnType(builderType) + .setArguments(settingsVarExpr.toBuilder().setIsDecl(true).build()) + .setBody( + ctorBodyExprs.stream() + .map(e -> ExprStatement.withExpr(e)) + .collect(Collectors.toList())) + .build()); + return ctorMethods; } 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 9c6c76eae8..4979b648a9 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 @@ -422,6 +422,22 @@ public void generateServiceClasses() { + " initDefaults(this);\n" + " }\n" + "\n" + + " protected Builder(EchoStubSettings settings) {\n" + + " super(settings);\n" + + " echoSettings = settings.echoSettings.toBuilder();\n" + + " expandSettings = settings.expandSettings.toBuilder();\n" + + " collectSettings = settings.collectSettings.toBuilder();\n" + + " chatSettings = settings.chatSettings.toBuilder();\n" + + " chatAgainSettings = settings.chatAgainSettings.toBuilder();\n" + + " pagedExpandSettings = settings.pagedExpandSettings.toBuilder();\n" + + " waitSettings = settings.waitSettings.toBuilder();\n" + + " waitOperationSettings = settings.waitOperationSettings.toBuilder();\n" + + " blockSettings = settings.blockSettings.toBuilder();\n" + + " unaryMethodSettingsBuilders =\n" + + " ImmutableList.>of(\n" + + " echoSettings, pagedExpandSettings, waitSettings, blockSettings);\n" + + " }\n" + + "\n" + " private static Builder initDefaults(Builder builder) {\n" + " builder\n" + " .echoSettings()\n" From 6858b33418961e18e5819f44ff0a058157f48c99 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 27 Aug 2020 14:22:50 -0700 Subject: [PATCH 18/24] feat: add createDefault() to ServiceStubSettings --- .../ServiceStubSettingsClassComposer.java | 88 +++++++++++++++++++ .../ServiceStubSettingsClassComposerTest.java | 10 +++ 2 files changed, 98 insertions(+) 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 42127311a3..0f7cd62f4d 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 @@ -1164,6 +1164,7 @@ private static List createNestedClassMethods( List nestedClassMethods = new ArrayList<>(); nestedClassMethods.addAll( createNestedClassConstructorMethods(service, nestedMethodSettingsMemberVarExprs, types)); + nestedClassMethods.add(createNestedClassCreateDefaultMethod(types)); nestedClassMethods.add(createNestedClassInitDefaultsMethod(service, serviceConfig, types)); // TODO(miraleung): More methods. @@ -1414,6 +1415,93 @@ private static List createNestedClassConstructorMethods( return ctorMethods; } + private static MethodDefinition createNestedClassCreateDefaultMethod( + Map types) { + List bodyExprs = new ArrayList<>(); + + // Initialize the builder: Builder builder = new Builder((ClientContext) null); + TypeNode builderType = types.get(NESTED_BUILDER_CLASS_NAME); + VariableExpr builderVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(builderType).setName("builder").build()); + bodyExprs.add( + AssignmentExpr.builder() + .setVariableExpr(builderVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr( + NewObjectExpr.builder() + .setType(builderType) + .setArguments( + CastExpr.builder() + .setType(STATIC_TYPES.get("ClientContext")) + .setExpr(ValueExpr.withValue(NullObjectValue.create())) + .build()) + .build()) + .build()); + + bodyExprs.add( + MethodInvocationExpr.builder() + .setExprReferenceExpr(builderVarExpr) + .setMethodName("setTransportChannelProvider") + .setArguments( + MethodInvocationExpr.builder() + .setMethodName("defaultTransportChannelProvider") + .build()) + .build()); + + bodyExprs.add( + MethodInvocationExpr.builder() + .setExprReferenceExpr(builderVarExpr) + .setMethodName("setCredentialsProvider") + .setArguments( + MethodInvocationExpr.builder() + .setExprReferenceExpr( + MethodInvocationExpr.builder() + .setMethodName("defaultCredentialsProviderBuilder") + .build()) + .setMethodName("build") + .build()) + .build()); + + bodyExprs.add( + MethodInvocationExpr.builder() + .setExprReferenceExpr(builderVarExpr) + .setMethodName("setInternalHeaderProvider") + .setArguments( + MethodInvocationExpr.builder() + .setExprReferenceExpr( + MethodInvocationExpr.builder() + .setMethodName("defaultApiClientHeaderProviderBuilder") + .build()) + .setMethodName("build") + .build()) + .build()); + + bodyExprs.add( + MethodInvocationExpr.builder() + .setExprReferenceExpr(builderVarExpr) + .setMethodName("setEndpoint") + .setArguments( + MethodInvocationExpr.builder().setMethodName("getDefaultEndpoint").build()) + .build()); + + Expr returnExpr = + MethodInvocationExpr.builder() + .setMethodName("initDefaults") + .setArguments(builderVarExpr) + .setReturnType(builderType) + .build(); + + return MethodDefinition.builder() + .setScope(ScopeNode.PRIVATE) + .setIsStatic(true) + .setReturnType(builderType) + .setName("createDefault") + .setBody( + bodyExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList())) + .setReturnExpr(returnExpr) + .build(); + } + private static Map createStaticTypes() { List concreteClazzes = Arrays.asList( 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 4979b648a9..0924a85ec7 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 @@ -438,6 +438,16 @@ public void generateServiceClasses() { + " echoSettings, pagedExpandSettings, waitSettings, blockSettings);\n" + " }\n" + "\n" + + " private static Builder createDefault() {\n" + + " Builder builder = new Builder(((ClientContext) null));\n" + + " builder.setTransportChannelProvider(defaultTransportChannelProvider());\n" + + " builder.setCredentialsProvider(defaultCredentialsProviderBuilder().build());\n" + + " " + + " builder.setInternalHeaderProvider(defaultApiClientHeaderProviderBuilder().build());\n" + + " builder.setEndpoint(getDefaultEndpoint());\n" + + " return initDefaults(builder);\n" + + " }\n" + + "\n" + " private static Builder initDefaults(Builder builder) {\n" + " builder\n" + " .echoSettings()\n" From 0df98cbb9d1ead34d233cad0c17ab320dec5df71 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 27 Aug 2020 14:39:15 -0700 Subject: [PATCH 19/24] feat: add ServiceStubSettings.applyToAllUnaryMethods method --- .../ServiceStubSettingsClassComposer.java | 46 ++++++++++++++++++- .../ServiceStubSettingsClassComposerTest.java | 8 ++++ 2 files changed, 52 insertions(+), 2 deletions(-) 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 0f7cd62f4d..8008caf95d 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 @@ -70,6 +70,7 @@ import com.google.api.generator.engine.ast.ScopeNode; import com.google.api.generator.engine.ast.Statement; import com.google.api.generator.engine.ast.StringObjectValue; +import com.google.api.generator.engine.ast.SuperObjectValue; import com.google.api.generator.engine.ast.TernaryExpr; import com.google.api.generator.engine.ast.ThisObjectValue; import com.google.api.generator.engine.ast.ThrowExpr; @@ -1102,7 +1103,7 @@ private static ClassDefinition createNestedBuilderClass( createNestedClassStatements(service, serviceConfig, nestedMethodSettingsMemberVarExprs)) .setMethods( createNestedClassMethods( - service, serviceConfig, nestedMethodSettingsMemberVarExprs, types)) + service, serviceConfig, extendsType, nestedMethodSettingsMemberVarExprs, types)) .build(); } @@ -1159,6 +1160,7 @@ private static List createNestedClassStatements( private static List createNestedClassMethods( Service service, GapicServiceConfig serviceConfig, + TypeNode superType, Map nestedMethodSettingsMemberVarExprs, Map types) { List nestedClassMethods = new ArrayList<>(); @@ -1166,6 +1168,7 @@ private static List createNestedClassMethods( createNestedClassConstructorMethods(service, nestedMethodSettingsMemberVarExprs, types)); nestedClassMethods.add(createNestedClassCreateDefaultMethod(types)); nestedClassMethods.add(createNestedClassInitDefaultsMethod(service, serviceConfig, types)); + nestedClassMethods.add(createNestedClassApplyToAllUnaryMethodsMethod(superType, types)); // TODO(miraleung): More methods. return nestedClassMethods; @@ -1502,6 +1505,46 @@ private static MethodDefinition createNestedClassCreateDefaultMethod( .build(); } + private static MethodDefinition createNestedClassApplyToAllUnaryMethodsMethod( + TypeNode superType, Map types) { + List apiFunctionTypeGenerics = new ArrayList<>(); + apiFunctionTypeGenerics.addAll( + NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR.type().reference().generics()); + apiFunctionTypeGenerics.add(TypeNode.VOID_OBJECT.reference()); + + TypeNode settingsUpdaterType = + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(ApiFunction.class) + .setGenerics(apiFunctionTypeGenerics) + .build()); + VariableExpr settingsUpdaterVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(settingsUpdaterType).setName("settingsUpdater").build()); + + String methodName = "applyToAllUnaryMethods"; + Expr superApplyExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(ValueExpr.withValue(SuperObjectValue.withType(superType))) + .setMethodName(methodName) + .setArguments(NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR, settingsUpdaterVarExpr) + .build(); + + TypeNode returnType = types.get(NESTED_BUILDER_CLASS_NAME); + Expr returnExpr = ValueExpr.withValue(ThisObjectValue.withType(returnType)); + + // TODO(miraleung): Add major ver note. + return MethodDefinition.builder() + .setScope(ScopeNode.PUBLIC) + .setReturnType(returnType) + .setName(methodName) + .setArguments(settingsUpdaterVarExpr.toBuilder().setIsDecl(true).build()) + .setThrowsExceptions(Arrays.asList(TypeNode.withExceptionClazz(Exception.class))) + .setBody(Arrays.asList(ExprStatement.withExpr(superApplyExpr))) + .setReturnExpr(returnExpr) + .build(); + } + private static Map createStaticTypes() { List concreteClazzes = Arrays.asList( @@ -1609,7 +1652,6 @@ private static Map createDynamicTypes(Service service, String .setIsStaticImport(true) .build())))); - // TODO(miraleung): Fill this out. return dynamicTypes; } 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 0924a85ec7..edad53953c 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 @@ -82,6 +82,7 @@ public void generateServiceClasses() { + "\n" + "import static com.google.showcase.v1beta1.EchoClient.PagedExpandPagedResponse;\n" + "\n" + + "import com.google.api.core.ApiFunction;\n" + "import com.google.api.core.ApiFuture;\n" + "import com.google.api.core.BetaApi;\n" + "import com.google.api.gax.core.GaxProperties;\n" @@ -504,6 +505,13 @@ public void generateServiceClasses() { + " .build()));\n" + " return builder;\n" + " }\n" + + "\n" + + " public Builder applyToAllUnaryMethods(\n" + + " ApiFunction, Void> settingsUpdater) throws" + + " Exception {\n" + + " super.applyToAllUnaryMethods(unaryMethodSettingsBuilders, settingsUpdater);\n" + + " return this;\n" + + " }\n" + " }\n" + "}\n"; } From 5919b2947c791908f5736f4f07c1968732f73871 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 27 Aug 2020 14:42:19 -0700 Subject: [PATCH 20/24] feat: add ServiceStubSettings.unaryMethodSettingsBuilders() --- .../composer/ServiceStubSettingsClassComposer.java | 10 ++++++++++ .../composer/ServiceStubSettingsClassComposerTest.java | 5 +++++ 2 files changed, 15 insertions(+) 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 8008caf95d..bfff0971ef 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 @@ -1169,6 +1169,7 @@ private static List createNestedClassMethods( nestedClassMethods.add(createNestedClassCreateDefaultMethod(types)); nestedClassMethods.add(createNestedClassInitDefaultsMethod(service, serviceConfig, types)); nestedClassMethods.add(createNestedClassApplyToAllUnaryMethodsMethod(superType, types)); + nestedClassMethods.add(createNestedClassUnaryMethodSettingsBuilderGetterMethod()); // TODO(miraleung): More methods. return nestedClassMethods; @@ -1545,6 +1546,15 @@ private static MethodDefinition createNestedClassApplyToAllUnaryMethodsMethod( .build(); } + private static MethodDefinition createNestedClassUnaryMethodSettingsBuilderGetterMethod() { + return MethodDefinition.builder() + .setScope(ScopeNode.PUBLIC) + .setReturnType(NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR.type()) + .setName("unaryMethodSettingsBuilders") + .setReturnExpr(NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR) + .build(); + } + private static Map createStaticTypes() { List concreteClazzes = Arrays.asList( 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 edad53953c..ed0ac3ba6a 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 @@ -512,6 +512,11 @@ public void generateServiceClasses() { + " super.applyToAllUnaryMethods(unaryMethodSettingsBuilders, settingsUpdater);\n" + " return this;\n" + " }\n" + + "\n" + + " public ImmutableList>" + + " unaryMethodSettingsBuilders() {\n" + + " return unaryMethodSettingsBuilders;\n" + + " }\n" + " }\n" + "}\n"; } From 0e03f06e1e8aef4dc919e4e99e175525afa34cd8 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 27 Aug 2020 14:46:32 -0700 Subject: [PATCH 21/24] feat: add ServiceStubSettings.build() --- .../ServiceStubSettingsClassComposer.java | 20 +++++++++++++++++++ .../ServiceStubSettingsClassComposerTest.java | 5 +++++ 2 files changed, 25 insertions(+) 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 bfff0971ef..9076b4c219 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 @@ -1172,6 +1172,7 @@ private static List createNestedClassMethods( nestedClassMethods.add(createNestedClassUnaryMethodSettingsBuilderGetterMethod()); // TODO(miraleung): More methods. + nestedClassMethods.add(createNestedClassBuildMethod(service, types)); return nestedClassMethods; } @@ -1555,6 +1556,25 @@ private static MethodDefinition createNestedClassUnaryMethodSettingsBuilderGette .build(); } + private static MethodDefinition createNestedClassBuildMethod( + Service service, Map types) { + TypeNode outerClassType = types.get(getThisClassName(service.name())); + TypeNode builderType = types.get(NESTED_BUILDER_CLASS_NAME); + + return MethodDefinition.builder() + .setIsOverride(true) + .setScope(ScopeNode.PUBLIC) + .setReturnType(outerClassType) + .setName("build") + .setThrowsExceptions(Arrays.asList(TypeNode.withExceptionClazz(IOException.class))) + .setReturnExpr( + NewObjectExpr.builder() + .setType(outerClassType) + .setArguments(ValueExpr.withValue(ThisObjectValue.withType(builderType))) + .build()) + .build(); + } + private static Map createStaticTypes() { List concreteClazzes = Arrays.asList( 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 ed0ac3ba6a..50f978ef1f 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 @@ -517,6 +517,11 @@ public void generateServiceClasses() { + " unaryMethodSettingsBuilders() {\n" + " return unaryMethodSettingsBuilders;\n" + " }\n" + + "\n" + + " @Override\n" + + " public EchoStubSettings build() throws IOException {\n" + + " return new EchoStubSettings(this);\n" + + " }\n" + " }\n" + "}\n"; } From f172ad16f9d549c3b8131cbef23bb0bb7d5413b9 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 27 Aug 2020 14:56:43 -0700 Subject: [PATCH 22/24] feat: add settingsBuilder getters in ServiceStubSettings --- .../ServiceStubSettingsClassComposer.java | 41 ++++++++++++++++- .../ServiceStubSettingsClassComposerTest.java | 46 +++++++++++++++++++ 2 files changed, 85 insertions(+), 2 deletions(-) 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 9076b4c219..3aaa35301d 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 @@ -1170,8 +1170,8 @@ private static List createNestedClassMethods( nestedClassMethods.add(createNestedClassInitDefaultsMethod(service, serviceConfig, types)); nestedClassMethods.add(createNestedClassApplyToAllUnaryMethodsMethod(superType, types)); nestedClassMethods.add(createNestedClassUnaryMethodSettingsBuilderGetterMethod()); - - // TODO(miraleung): More methods. + nestedClassMethods.addAll( + createNestedClassSettingsBuilderGetterMethods(nestedMethodSettingsMemberVarExprs)); nestedClassMethods.add(createNestedClassBuildMethod(service, types)); return nestedClassMethods; } @@ -1556,6 +1556,43 @@ private static MethodDefinition createNestedClassUnaryMethodSettingsBuilderGette .build(); } + private static List createNestedClassSettingsBuilderGetterMethods( + Map nestedMethodSettingsMemberVarExprs) { + Reference operationCallSettingsBuilderRef = + ConcreteReference.withClazz(OperationCallSettings.Builder.class); + Function isOperationCallSettingsBuilderFn = + t -> + t.reference() + .copyAndSetGenerics(ImmutableList.of()) + .equals(operationCallSettingsBuilderRef); + List lroBetaAnnotations = + Arrays.asList( + AnnotationNode.builder() + .setType(STATIC_TYPES.get("BetaApi")) + .setDescription( + "The surface for use by generated code is not stable yet and may change in the" + + " future.") + .build()); + + List javaMethods = new ArrayList<>(); + for (Map.Entry settingsVarEntry : + nestedMethodSettingsMemberVarExprs.entrySet()) { + String varName = settingsVarEntry.getKey(); + VariableExpr settingsVarExpr = settingsVarEntry.getValue(); + boolean isOperationCallSettings = + isOperationCallSettingsBuilderFn.apply(settingsVarExpr.type()); + javaMethods.add( + MethodDefinition.builder() + .setAnnotations(isOperationCallSettings ? lroBetaAnnotations : ImmutableList.of()) + .setScope(ScopeNode.PUBLIC) + .setReturnType(settingsVarExpr.type()) + .setName(settingsVarExpr.variable().identifier().name()) + .setReturnExpr(settingsVarExpr) + .build()); + } + return javaMethods; + } + private static MethodDefinition createNestedClassBuildMethod( Service service, Map types) { TypeNode outerClassType = types.get(getThisClassName(service.name())); 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 50f978ef1f..75cb1b3275 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 @@ -518,6 +518,52 @@ public void generateServiceClasses() { + " return unaryMethodSettingsBuilders;\n" + " }\n" + "\n" + + " public UnaryCallSettings.Builder echoSettings() {\n" + + " return echoSettings;\n" + + " }\n" + + "\n" + + " public ServerStreamingCallSettings.Builder" + + " expandSettings() {\n" + + " return expandSettings;\n" + + " }\n" + + "\n" + + " public StreamingCallSettings.Builder collectSettings()" + + " {\n" + + " return collectSettings;\n" + + " }\n" + + "\n" + + " public StreamingCallSettings.Builder chatSettings()" + + " {\n" + + " return chatSettings;\n" + + " }\n" + + "\n" + + " public StreamingCallSettings.Builder" + + " chatAgainSettings() {\n" + + " return chatAgainSettings;\n" + + " }\n" + + "\n" + + " public PagedCallSettings.Builder<\n" + + " PagedExpandRequest, PagedExpandResponse, PagedExpandPagedResponse>\n" + + " pagedExpandSettings() {\n" + + " return pagedExpandSettings;\n" + + " }\n" + + "\n" + + " public UnaryCallSettings.Builder waitSettings() {\n" + + " return waitSettings;\n" + + " }\n" + + "\n" + + " @BetaApi(\n" + + " \"The surface for use by generated code is not stable yet and may change in" + + " the future.\")\n" + + " public OperationCallSettings.Builder\n" + + " waitOperationSettings() {\n" + + " return waitOperationSettings;\n" + + " }\n" + + "\n" + + " public UnaryCallSettings.Builder blockSettings() {\n" + + " return blockSettings;\n" + + " }\n" + + "\n" + " @Override\n" + " public EchoStubSettings build() throws IOException {\n" + " return new EchoStubSettings(this);\n" From de8ab3b199faa29db3cfb2d18b781bad5795b9ae Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 28 Aug 2020 13:56:54 -0700 Subject: [PATCH 23/24] feat: add gapic.yaml batching parsing --- dependencies.properties | 5 +- .../gapic/model/GapicBatchingSettings.java | 76 +++++ .../generator/gapic/protoparser/BUILD.bazel | 1 + .../BatchingSettingsConfigParser.java | 147 +++++++++ .../generator/gapic/protoparser/Parser.java | 2 +- .../generator/gapic/protoparser/BUILD.bazel | 2 + .../BatchingSettingsConfigParserTest.java | 106 +++++++ .../api/generator/gapic/testdata/BUILD.bazel | 5 + .../gapic/testdata/datastore_gapic.yaml | 38 +++ .../gapic/testdata/logging_gapic.yaml | 79 +++++ .../gapic/testdata/pubsub_gapic.yaml | 296 ++++++++++++++++++ .../gapic/testdata/showcase_gapic.yaml | 22 ++ 12 files changed, 777 insertions(+), 2 deletions(-) create mode 100644 src/main/java/com/google/api/generator/gapic/model/GapicBatchingSettings.java create mode 100644 src/main/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParser.java create mode 100644 src/test/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParserTest.java create mode 100644 src/test/java/com/google/api/generator/gapic/testdata/datastore_gapic.yaml create mode 100644 src/test/java/com/google/api/generator/gapic/testdata/logging_gapic.yaml create mode 100644 src/test/java/com/google/api/generator/gapic/testdata/pubsub_gapic.yaml create mode 100644 src/test/java/com/google/api/generator/gapic/testdata/showcase_gapic.yaml diff --git a/dependencies.properties b/dependencies.properties index 2fdf5025d0..05ca128d8f 100644 --- a/dependencies.properties +++ b/dependencies.properties @@ -22,7 +22,10 @@ maven.com_google_auto_value_auto_value=com.google.auto.value:auto-value:1.7.2 maven.com_google_auto_value_auto_value_annotations=com.google.auto.value:auto-value-annotations:1.7.2 maven.com_google_protobuf_protobuf_java=com.google.protobuf:protobuf-java:3.12.2 -# ServiceStubSettings class. +# Gapic YAML parsing for batching settings. +maven.org_yaml_snakeyaml=org.yaml:snakeyaml:1.26 + +# ServiceStubSettings class. Used only in generated code. maven.org_threeten_threetenbp=org.threeten:threetenbp:1.3.3 # Testing. 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 new file mode 100644 index 0000000000..04085ab5f4 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/model/GapicBatchingSettings.java @@ -0,0 +1,76 @@ +// 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.model; + +import com.google.auto.value.AutoValue; +import javax.annotation.Nullable; + +@AutoValue +public abstract class GapicBatchingSettings { + public enum FlowControlLimitExceededBehavior { + THROW_EXCEPTION, + BLOCK, + IGNORE + }; + + public abstract String protoPakkage(); + + public abstract String serviceName(); + + public abstract String methodName(); + + public abstract int elementCountThreshold(); + + public abstract long requestByteThreshold(); + + public abstract long delayThresholdMillis(); + + @Nullable + public abstract Integer flowControlElementLimit(); + + @Nullable + public abstract Integer flowControlByteLimit(); + + public abstract FlowControlLimitExceededBehavior flowControlLimitExceededBehavior(); + + public static Builder builder() { + return new AutoValue_GapicBatchingSettings.Builder() + .setFlowControlLimitExceededBehavior(FlowControlLimitExceededBehavior.IGNORE); + } + + @AutoValue.Builder + public abstract static class Builder { + public abstract Builder setProtoPakkage(String protoPakkage); + + public abstract Builder setServiceName(String serviceName); + + public abstract Builder setMethodName(String methodName); + + public abstract Builder setElementCountThreshold(int elementCountThreshold); + + public abstract Builder setRequestByteThreshold(long requestByteThreshold); + + public abstract Builder setDelayThresholdMillis(long delalyThresholdMillis); + + public abstract Builder setFlowControlElementLimit(Integer flowControlElementLimit); + + public abstract Builder setFlowControlByteLimit(Integer flowControlByteLimit); + + public abstract Builder setFlowControlLimitExceededBehavior( + FlowControlLimitExceededBehavior behavior); + + public abstract GapicBatchingSettings build(); + } +} diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/BUILD.bazel b/src/main/java/com/google/api/generator/gapic/protoparser/BUILD.bazel index e6b95b32be..412a924667 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/BUILD.bazel +++ b/src/main/java/com/google/api/generator/gapic/protoparser/BUILD.bazel @@ -32,5 +32,6 @@ java_library( "@com_google_guava_guava//jar", "@com_google_protobuf//:protobuf_java", "@com_google_protobuf//:protobuf_java_util", + "@org_yaml_snakeyaml//jar", ], ) 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 new file mode 100644 index 0000000000..ce55d25750 --- /dev/null +++ b/src/main/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParser.java @@ -0,0 +1,147 @@ +// 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.protoparser; + +import com.google.api.generator.gapic.model.GapicBatchingSettings; +import com.google.common.base.Preconditions; +import com.google.common.base.Strings; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import org.yaml.snakeyaml.Yaml; +import org.yaml.snakeyaml.constructor.SafeConstructor; + +public class BatchingSettingsConfigParser { + private static String LIMIT_EXCEEDED_BEHAVIOR_THROW_EXCEPTION_YAML_VALUE = "THROW_EXCEPTION"; + private static String LIMIT_EXCEEDED_BEHAVIOR_BLOCK_YAML_VALUE = "BLOCK"; + + private static String YAML_KEY_INTERFACES = "interfaces"; + private static String YAML_KEY_NAME = "name"; + 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_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"; + private static String YAML_KEY_BATCHING_FLOW_CONTROL_ELEMENT_LIMIT = "flow_control_element_limit"; + private static String YAML_KEY_BATCHING_FLOW_CONTROL_BYTE_LIMIT = "flow_control_byte_limit"; + private static String YAML_KEY_BATCHING_FLOW_CONTROL_LIMIT_EXCEEDED_BEHAVIOR = + "flow_control_limit_exceeded_behavior"; + + public static Optional> parse(String gapicYamlConfigFilePath) { + if (Strings.isNullOrEmpty(gapicYamlConfigFilePath) + || !(new File(gapicYamlConfigFilePath)).exists()) { + return Optional.empty(); + } + + String fileContents = null; + + try { + fileContents = new String(Files.readAllBytes(Paths.get(gapicYamlConfigFilePath))); + } catch (IOException e) { + return Optional.empty(); + } + + Yaml yaml = new Yaml(new SafeConstructor()); + Map yamlMap = yaml.load(fileContents); + return parseFromMap(yamlMap); + } + + private static Optional> parseFromMap(Map yamlMap) { + if (!yamlMap.containsKey(YAML_KEY_INTERFACES)) { + return Optional.empty(); + } + + List settings = new ArrayList<>(); + for (Map serviceYamlConfig : + (List>) yamlMap.get(YAML_KEY_INTERFACES)) { + if (!serviceYamlConfig.containsKey(YAML_KEY_METHODS)) { + continue; + } + for (Map methodYamlConfig : + (List>) serviceYamlConfig.get(YAML_KEY_METHODS)) { + if (!methodYamlConfig.containsKey(YAML_KEY_BATCHING)) { + continue; + } + Map batchingOuterYamlConfig = + (Map) methodYamlConfig.get(YAML_KEY_BATCHING); + if (!batchingOuterYamlConfig.containsKey(YAML_KEY_THRESHOLDS)) { + continue; + } + Map batchingYamlConfig = + (Map) batchingOuterYamlConfig.get(YAML_KEY_THRESHOLDS); + Preconditions.checkState( + batchingYamlConfig.containsKey(YAML_KEY_BATCHING_ELEMENT_COUNT_THRESHOLD) + && batchingYamlConfig.containsKey(YAML_KEY_BATCHING_REQUEST_BYTE_THRESHOLD) + && batchingYamlConfig.containsKey(YAML_KEY_BATCHING_DELAY_THRESHOLD_MILLIS), + String.format( + "Batching YAML config is missing one of %s, %s, or %s fields", + YAML_KEY_BATCHING_ELEMENT_COUNT_THRESHOLD, + YAML_KEY_BATCHING_REQUEST_BYTE_THRESHOLD, + YAML_KEY_BATCHING_DELAY_THRESHOLD_MILLIS)); + + String interfaceName = (String) serviceYamlConfig.get(YAML_KEY_NAME); + int lastDotIndex = interfaceName.lastIndexOf("."); + String protoPakkage = interfaceName.substring(0, lastDotIndex); + String serviceName = interfaceName.substring(lastDotIndex + 1); + String methodName = (String) methodYamlConfig.get(YAML_KEY_NAME); + GapicBatchingSettings.Builder settingsBuilder = + GapicBatchingSettings.builder() + .setProtoPakkage(protoPakkage) + .setServiceName(serviceName) + .setMethodName(methodName) + .setElementCountThreshold( + (Integer) batchingYamlConfig.get(YAML_KEY_BATCHING_ELEMENT_COUNT_THRESHOLD)) + .setRequestByteThreshold( + (Integer) batchingYamlConfig.get(YAML_KEY_BATCHING_REQUEST_BYTE_THRESHOLD)) + .setDelayThresholdMillis( + (Integer) batchingYamlConfig.get(YAML_KEY_BATCHING_DELAY_THRESHOLD_MILLIS)); + + if (batchingYamlConfig.containsKey(YAML_KEY_BATCHING_FLOW_CONTROL_ELEMENT_LIMIT)) { + settingsBuilder.setFlowControlElementLimit( + (Integer) batchingYamlConfig.get(YAML_KEY_BATCHING_FLOW_CONTROL_ELEMENT_LIMIT)); + } + if (batchingYamlConfig.containsKey(YAML_KEY_BATCHING_FLOW_CONTROL_BYTE_LIMIT)) { + settingsBuilder.setFlowControlByteLimit( + (Integer) batchingYamlConfig.get(YAML_KEY_BATCHING_FLOW_CONTROL_BYTE_LIMIT)); + } + if (batchingYamlConfig.containsKey( + YAML_KEY_BATCHING_FLOW_CONTROL_LIMIT_EXCEEDED_BEHAVIOR)) { + String behaviorYamlValue = + (String) + batchingYamlConfig.get(YAML_KEY_BATCHING_FLOW_CONTROL_LIMIT_EXCEEDED_BEHAVIOR); + GapicBatchingSettings.FlowControlLimitExceededBehavior behaviorSetting = + GapicBatchingSettings.FlowControlLimitExceededBehavior.IGNORE; + // IGNORE or UNSET_BEHAVIOR YAML values map to FlowControlLimitExceededBehavior.IGNOER. + if (behaviorYamlValue.equals(LIMIT_EXCEEDED_BEHAVIOR_THROW_EXCEPTION_YAML_VALUE)) { + behaviorSetting = + GapicBatchingSettings.FlowControlLimitExceededBehavior.THROW_EXCEPTION; + } else if (behaviorYamlValue.equals(LIMIT_EXCEEDED_BEHAVIOR_BLOCK_YAML_VALUE)) { + behaviorSetting = GapicBatchingSettings.FlowControlLimitExceededBehavior.BLOCK; + } + settingsBuilder.setFlowControlLimitExceededBehavior(behaviorSetting); + } + settings.add(settingsBuilder.build()); + } + } + + return settings.isEmpty() ? Optional.empty() : Optional.of(settings); + } +} 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 dd80c26b69..42c64e6d9f 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 @@ -74,7 +74,7 @@ public static GapicContext parse(CodeGeneratorRequest request) { String serviceConfigPath = serviceConfigPathOpt.isPresent() ? serviceConfigPathOpt.get() : null; Optional serviceConfigOpt = ServiceConfigParser.parse(serviceConfigPath); - // TODO(miraleung): Actually parse the yaml file. + // TODO(miraleung): Actually pars the yaml file. Optional gapicYamlConfigPathOpt = PluginArgumentParser.parseGapicYamlConfigPath(request); String gapicYamlConfigPath = diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel index 199099fc25..30dbd181fe 100644 --- a/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel @@ -1,6 +1,7 @@ package(default_visibility = ["//visibility:public"]) TESTS = [ + "BatchingSettingsConfigParserTest", "MethodSignatureParserTest", "ParserTest", "PluginArgumentParserTest", @@ -18,6 +19,7 @@ filegroup( name = test_name, srcs = ["{0}.java".format(test_name)], data = [ + "//src/test/java/com/google/api/generator/gapic/testdata:gapic_config_files", "//src/test/java/com/google/api/generator/gapic/testdata:service_config_files", ], test_class = "com.google.api.generator.gapic.protoparser.{0}".format(test_name), 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 new file mode 100644 index 0000000000..ed420d87a2 --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParserTest.java @@ -0,0 +1,106 @@ +// 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.protoparser; + +import static com.google.common.truth.Truth.assertThat; +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.assertTrue; + +import com.google.api.generator.gapic.model.GapicBatchingSettings; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.List; +import java.util.Optional; +import org.junit.Test; + +public class BatchingSettingsConfigParserTest { + private static final String YAML_DIRECTORY = + "src/test/java/com/google/api/generator/gapic/testdata/"; + + @Test + public void parseGapicSettings_plain() { + String filename = "datastore_gapic.yaml"; + Path path = Paths.get(YAML_DIRECTORY, filename); + Optional> settingsOpt = + BatchingSettingsConfigParser.parse(path.toString()); + assertFalse(settingsOpt.isPresent()); + } + + @Test + public void parseGapicSettings_noMethodSettings() { + String filename = "showcase_gapic.yaml"; + Path path = Paths.get(YAML_DIRECTORY, filename); + Optional> settingsOpt = + BatchingSettingsConfigParser.parse(path.toString()); + assertFalse(settingsOpt.isPresent()); + } + + @Test + public void parseBatchingSettings_logging() { + String filename = "logging_gapic.yaml"; + Path path = Paths.get(YAML_DIRECTORY, filename); + Optional> settingsOpt = + BatchingSettingsConfigParser.parse(path.toString()); + assertTrue(settingsOpt.isPresent()); + + List batchingSettings = settingsOpt.get(); + assertEquals(1, batchingSettings.size()); + GapicBatchingSettings setting = batchingSettings.get(0); + + assertEquals("google.logging.v2", setting.protoPakkage()); + assertEquals("LoggingServiceV2", setting.serviceName()); + assertEquals("WriteLogEntries", setting.methodName()); + + assertEquals(1000, setting.elementCountThreshold()); + assertEquals(1048576, setting.requestByteThreshold()); + assertEquals(50, setting.delayThresholdMillis()); + + assertThat(setting.flowControlElementLimit()).isNotNull(); + assertEquals(100000, (long) setting.flowControlElementLimit()); + assertThat(setting.flowControlByteLimit()).isNotNull(); + assertEquals(10485760, (long) setting.flowControlByteLimit()); + assertEquals( + GapicBatchingSettings.FlowControlLimitExceededBehavior.THROW_EXCEPTION, + setting.flowControlLimitExceededBehavior()); + } + + @Test + public void parseBatchingSettings_pubsub() { + String filename = "pubsub_gapic.yaml"; + Path path = Paths.get(YAML_DIRECTORY, filename); + Optional> settingsOpt = + BatchingSettingsConfigParser.parse(path.toString()); + assertTrue(settingsOpt.isPresent()); + + List batchingSettings = settingsOpt.get(); + assertEquals(1, batchingSettings.size()); + GapicBatchingSettings setting = batchingSettings.get(0); + + assertEquals("google.pubsub.v1", setting.protoPakkage()); + assertEquals("Publisher", setting.serviceName()); + assertEquals("Publish", setting.methodName()); + + assertEquals(100, setting.elementCountThreshold()); + assertEquals(1048576, setting.requestByteThreshold()); + assertEquals(10, setting.delayThresholdMillis()); + + assertThat(setting.flowControlElementLimit()).isNull(); + assertThat(setting.flowControlByteLimit()).isNull(); + assertEquals( + GapicBatchingSettings.FlowControlLimitExceededBehavior.IGNORE, + setting.flowControlLimitExceededBehavior()); + } +} diff --git a/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel index 32260413ac..44d2b3bf82 100644 --- a/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel @@ -5,6 +5,11 @@ filegroup( srcs = glob(["*.json"]), ) +filegroup( + name = "gapic_config_files", + srcs = glob(["*_gapic.yaml"]), +) + proto_library( name = "showcase_proto", srcs = [ diff --git a/src/test/java/com/google/api/generator/gapic/testdata/datastore_gapic.yaml b/src/test/java/com/google/api/generator/gapic/testdata/datastore_gapic.yaml new file mode 100644 index 0000000000..e9860d5efe --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/testdata/datastore_gapic.yaml @@ -0,0 +1,38 @@ +type: com.google.api.codegen.ConfigProto +config_schema_version: 2.0.0 +language_settings: + java: + package_name: com.google.cloud.datastore.v1 + python: + package_name: google.cloud.datastore_v1.gapic + go: + package_name: cloud.google.com/go/datastore/apiv1 + csharp: + package_name: Google.Cloud.Datastore.V1 + release_level: GA + ruby: + package_name: Google::Cloud::Datastore::V1 + release_level: GA + php: + package_name: Google\Cloud\Datastore\V1 + nodejs: + package_name: datastore.v1 + domain_layer_location: google-cloud +interfaces: +- name: google.datastore.v1.Datastore + retry_params_def: + - name: default + initial_retry_delay_millis: 100 + retry_delay_multiplier: 1.3 + max_retry_delay_millis: 60000 + initial_rpc_timeout_millis: 60000 + rpc_timeout_multiplier: 1 + max_rpc_timeout_millis: 60000 + total_timeout_millis: 600000 + methods: + - name: Lookup + retry_codes_name: idempotent + - name: RunQuery + retry_codes_name: idempotent + - name: ReserveIds + retry_codes_name: idempotent diff --git a/src/test/java/com/google/api/generator/gapic/testdata/logging_gapic.yaml b/src/test/java/com/google/api/generator/gapic/testdata/logging_gapic.yaml new file mode 100644 index 0000000000..75c6deda26 --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/testdata/logging_gapic.yaml @@ -0,0 +1,79 @@ +type: com.google.api.codegen.ConfigProto +config_schema_version: 2.0.0 +language_settings: + java: + package_name: com.google.cloud.logging.v2 + interface_names: + google.logging.v2.ConfigServiceV2: Config + google.logging.v2.LoggingServiceV2: Logging + google.logging.v2.MetricsServiceV2: Metrics + python: + package_name: google.cloud.logging_v2.gapic + go: + package_name: cloud.google.com/go/logging/apiv2 + domain_layer_location: cloud.google.com/go/logging + csharp: + package_name: Google.Cloud.Logging.V2 + release_level: GA + ruby: + package_name: Google::Cloud::Logging::V2 + php: + package_name: Google\Cloud\Logging\V2 + nodejs: + package_name: logging.v2 + domain_layer_location: google-cloud +interfaces: +- name: google.logging.v2.ConfigServiceV2 + retry_codes_def: + - name: idempotent + retry_codes: + - UNAVAILABLE + - DEADLINE_EXCEEDED + - INTERNAL + methods: + - name: DeleteSink + retry_codes_name: idempotent + - name: UpdateSink + retry_codes_name: idempotent + - name: DeleteExclusion + retry_codes_name: idempotent +- name: google.logging.v2.MetricsServiceV2 + retry_codes_def: + - name: idempotent + retry_codes: + - UNAVAILABLE + - DEADLINE_EXCEEDED + - INTERNAL + methods: + - name: UpdateLogMetric + retry_codes_name: idempotent + - name: DeleteLogMetric + retry_codes_name: idempotent +- name: google.logging.v2.LoggingServiceV2 + retry_codes_def: + - name: idempotent + retry_codes: + - UNAVAILABLE + - DEADLINE_EXCEEDED + - INTERNAL + methods: + - name: DeleteLog + retry_codes_name: idempotent + - name: ListLogEntries + retry_codes_name: idempotent + - name: WriteLogEntries + retry_codes_name: idempotent + batching: + thresholds: + element_count_threshold: 1000 + request_byte_threshold: 1048576 + delay_threshold_millis: 50 + flow_control_element_limit: 100000 + flow_control_byte_limit: 10485760 + flow_control_limit_exceeded_behavior: THROW_EXCEPTION + batch_descriptor: + batched_field: entries + discriminator_fields: + - log_name + - resource + - labels diff --git a/src/test/java/com/google/api/generator/gapic/testdata/pubsub_gapic.yaml b/src/test/java/com/google/api/generator/gapic/testdata/pubsub_gapic.yaml new file mode 100644 index 0000000000..e26d535faf --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/testdata/pubsub_gapic.yaml @@ -0,0 +1,296 @@ +type: com.google.api.codegen.ConfigProto +config_schema_version: 2.0.0 +language_settings: + java: + package_name: com.google.cloud.pubsub.v1 + interface_names: + google.pubsub.v1.Publisher: TopicAdmin + google.pubsub.v1.Subscriber: SubscriptionAdmin + release_level: GA + python: + package_name: google.cloud.pubsub_v1.gapic + release_level: GA + go: + package_name: cloud.google.com/go/pubsub/apiv1 + domain_layer_location: cloud.google.com/go/pubsub + release_level: GA + csharp: + package_name: Google.Cloud.PubSub.V1 + interface_names: + google.pubsub.v1.Publisher: PublisherServiceApi + google.pubsub.v1.Subscriber: SubscriberServiceApi + release_level: GA + ruby: + package_name: Google::Cloud::PubSub::V1 + release_level: BETA + php: + package_name: Google\Cloud\PubSub\V1 + release_level: GA + nodejs: + package_name: pubsub.v1 + domain_layer_location: google-cloud + release_level: GA +collections: +# Language overrides are for backward-compatibility in Java. +- entity_name: snapshot + language_overrides: + - language: java + entity_name: project_snapshot +# Language overrides are for backward-compatibility in Java. +- entity_name: subscription + language_overrides: + - language: java + entity_name: project_subscription +interfaces: +- name: google.pubsub.v1.Subscriber + # Deprecated collections are for backward-compatibility in Ruby, PHP and Python + deprecated_collections: + - name_pattern: "projects/{project}/topics/{topic}" + entity_name: topic + lang_doc: + java: To retrieve messages from a subscription, see the Subscriber class. + retry_codes_def: + - name: idempotent + retry_codes: + - UNKNOWN + - ABORTED + - UNAVAILABLE + - name: non_idempotent + retry_codes: + - UNAVAILABLE + - name: streaming_pull + retry_codes: + - DEADLINE_EXCEEDED + - RESOURCE_EXHAUSTED + - ABORTED + - INTERNAL + - UNAVAILABLE + retry_params_def: + - name: default + initial_retry_delay_millis: 100 + retry_delay_multiplier: 1.3 + max_retry_delay_millis: 60000 # 60 seconds + initial_rpc_timeout_millis: 60000 # 60 seconds + rpc_timeout_multiplier: 1 + max_rpc_timeout_millis: 60000 # 60 seconds + total_timeout_millis: 600000 # 10 minutes + - name: messaging + initial_retry_delay_millis: 100 + retry_delay_multiplier: 1.3 + max_retry_delay_millis: 60000 # 60 seconds + initial_rpc_timeout_millis: 25000 # 25 seconds + rpc_timeout_multiplier: 1 + max_rpc_timeout_millis: 25000 # 25 seconds + total_timeout_millis: 600000 # 10 minutes + - name: streaming_messaging + initial_retry_delay_millis: 100 + retry_delay_multiplier: 1.3 + max_retry_delay_millis: 60000 # 60 seconds + initial_rpc_timeout_millis: 600000 # 10 minutes + rpc_timeout_multiplier: 1 + max_rpc_timeout_millis: 600000 # 10 minutes + total_timeout_millis: 600000 # 10 minutes + methods: + # TODO: remove the per method retry_codes_name field once + # https://github.com/googleapis/gapic-generator/issues/3137 + # is fixed + - name: CreateSubscription + retry_codes_name: idempotent + - name: GetSubscription + retry_codes_name: idempotent + - name: UpdateSubscription + retry_codes_name: non_idempotent + sample_code_init_fields: + - update_mask.paths[0]="ack_deadline_seconds" + - subscription.ack_deadline_seconds=42 + - name: ListSubscriptions + retry_codes_name: idempotent + - name: DeleteSubscription + retry_codes_name: non_idempotent + - name: GetSnapshot + surface_treatments: + - include_languages: + - go + - java + - csharp + - ruby + - nodejs + - python + - php + visibility: PACKAGE + - name: ModifyAckDeadline + retry_codes_name: non_idempotent + surface_treatments: + - include_languages: + - java + visibility: PACKAGE + - name: Acknowledge + retry_codes_name: non_idempotent + retry_params_name: messaging + surface_treatments: + - include_languages: + - java + visibility: PACKAGE + - name: Pull + retry_codes_name: idempotent + retry_params_name: messaging + surface_treatments: + - include_languages: + - java + visibility: PACKAGE + - name: StreamingPull + retry_codes_name: streaming_pull + retry_params_name: streaming_messaging + timeout_millis: 900000 + surface_treatments: + - include_languages: + - java + visibility: PACKAGE + - name: ModifyPushConfig + retry_codes_name: non_idempotent + - name: ListSnapshots + retry_codes_name: idempotent + - name: CreateSnapshot + retry_codes_name: non_idempotent + - name: UpdateSnapshot + retry_codes_name: non_idempotent + sample_code_init_fields: + - update_mask.paths[0]="expire_time" + - snapshot.expire_time.seconds=123456 + - name: DeleteSnapshot + retry_codes_name: non_idempotent + - name: Seek + retry_codes_name: idempotent + - name: SetIamPolicy + retry_codes_name: non_idempotent + reroute_to_grpc_interface: google.iam.v1.IAMPolicy + surface_treatments: + - include_languages: + - go + visibility: DISABLED + - name: GetIamPolicy + retry_codes_name: idempotent + reroute_to_grpc_interface: google.iam.v1.IAMPolicy + surface_treatments: + - include_languages: + - go + visibility: DISABLED + - name: TestIamPermissions + retry_codes_name: non_idempotent + reroute_to_grpc_interface: google.iam.v1.IAMPolicy + surface_treatments: + - include_languages: + - go + visibility: DISABLED +- name: google.pubsub.v1.Publisher + lang_doc: + java: To publish messages to a topic, see the Publisher class. + smoke_test: + method: ListTopics + init_fields: + - project%project=$PROJECT_ID + retry_codes_def: + - name: idempotent + retry_codes: + - UNKNOWN + - ABORTED + - UNAVAILABLE + - name: non_idempotent + retry_codes: + - UNAVAILABLE + - name: none + retry_codes: [] + - name: publish + retry_codes: + - ABORTED + - CANCELLED + - INTERNAL + - RESOURCE_EXHAUSTED + - UNKNOWN + - UNAVAILABLE + - DEADLINE_EXCEEDED + retry_params_def: + - name: default + initial_retry_delay_millis: 100 + retry_delay_multiplier: 1.3 + max_retry_delay_millis: 60000 # 60 seconds + initial_rpc_timeout_millis: 60000 # 60 seconds + rpc_timeout_multiplier: 1 + max_rpc_timeout_millis: 60000 # 60 seconds + total_timeout_millis: 600000 # 10 minutes + - name: messaging + initial_retry_delay_millis: 100 + retry_delay_multiplier: 1.3 + max_retry_delay_millis: 60000 # 60 seconds + initial_rpc_timeout_millis: 5000 # 5 seconds + rpc_timeout_multiplier: 1.3 + max_rpc_timeout_millis: 60000 # 60 seconds + total_timeout_millis: 60000 # 60 seconds + methods: + - name: CreateTopic + retry_codes_name: non_idempotent + - name: UpdateTopic + retry_codes_name: non_idempotent + - name: Publish + retry_codes_name: publish + retry_params_name: messaging + batching: + thresholds: + element_count_threshold: 100 + element_count_limit: 1000 # TO BE REMOVED LATER + request_byte_threshold: 1048576 # 1 MiB + request_byte_limit: 10485760 # TO BE REMOVED LATER + delay_threshold_millis: 10 + batch_descriptor: + batched_field: messages + discriminator_fields: + - topic + subresponse_field: message_ids + sample_code_init_fields: + - messages[0].data + surface_treatments: + - include_languages: + - java + visibility: PACKAGE + - name: GetTopic + retry_codes_name: idempotent + - name: ListTopics + retry_codes_name: idempotent + - name: ListTopicSubscriptions + retry_codes_name: idempotent + - name: ListTopicSnapshots + surface_treatments: + - include_languages: + - go + - java + - csharp + - ruby + - nodejs + - python + - php + visibility: PACKAGE + - name: DeleteTopic + retry_codes_name: non_idempotent + - name: SetIamPolicy + retry_codes_name: non_idempotent + reroute_to_grpc_interface: google.iam.v1.IAMPolicy + # TODO: surface_treatments are here only to make bazel presubmit pass + # they can be removed once presubmit starts using gapic-generator-go + surface_treatments: + - include_languages: + - go + visibility: DISABLED + - name: GetIamPolicy + retry_codes_name: idempotent + reroute_to_grpc_interface: google.iam.v1.IAMPolicy + surface_treatments: + - include_languages: + - go + visibility: DISABLED + - name: TestIamPermissions + retry_codes_name: non_idempotent + reroute_to_grpc_interface: google.iam.v1.IAMPolicy + surface_treatments: + - include_languages: + - go + visibility: DISABLED diff --git a/src/test/java/com/google/api/generator/gapic/testdata/showcase_gapic.yaml b/src/test/java/com/google/api/generator/gapic/testdata/showcase_gapic.yaml new file mode 100644 index 0000000000..8c50128718 --- /dev/null +++ b/src/test/java/com/google/api/generator/gapic/testdata/showcase_gapic.yaml @@ -0,0 +1,22 @@ +# FIXME: Address all the FIXMEs in this generated config before using it for +# client generation. Remove this paragraph after you closed all the FIXMEs. The +# retry_codes_name, required_fields, flattening, and timeout properties cannot +# be precisely decided by the tooling and may require some configuration. +type: com.google.api.codegen.ConfigProto +config_schema_version: 2.0.0 +# The settings of generated code in a specific language. +language_settings: + java: + package_name: com.google.showcase.v1beta1 + python: + package_name: google.showcase_v1beta1.gapic + go: + package_name: cloud.google.com/go/showcase/apiv1beta1 + csharp: + package_name: Google.Showcase.V1beta1 + ruby: + package_name: Google::Showcase::V1beta1 + php: + package_name: Google\Showcase\V1beta1 + nodejs: + package_name: showcase.v1beta1 From 62331681c47379ab056ee41731ac08ed76fb8006 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 28 Aug 2020 14:46:43 -0700 Subject: [PATCH 24/24] feat: integrate batching with retry settings parsing --- .../gapic/model/GapicBatchingSettings.java | 6 ++ .../gapic/model/GapicServiceConfig.java | 82 +++++++++++++------ .../BatchingSettingsConfigParser.java | 9 +- .../generator/gapic/protoparser/Parser.java | 15 ++-- .../protoparser/ServiceConfigParser.java | 7 +- .../composer/RetrySettingsComposerTest.java | 18 ++-- .../ServiceStubSettingsClassComposerTest.java | 3 +- .../gapic/model/GapicServiceConfigTest.java | 82 ++++++++++++++++++- 8 files changed, 178 insertions(+), 44 deletions(-) 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 04085ab5f4..349cb479ab 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 @@ -45,6 +45,12 @@ public enum FlowControlLimitExceededBehavior { public abstract FlowControlLimitExceededBehavior flowControlLimitExceededBehavior(); + public boolean matches(Service service, Method method) { + return protoPakkage().equals(service.protoPakkage()) + && serviceName().equals(service.name()) + && methodName().equals(method.name()); + } + public static Builder builder() { return new AutoValue_GapicBatchingSettings.Builder() .setFlowControlLimitExceededBehavior(FlowControlLimitExceededBehavior.IGNORE); diff --git a/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java b/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java index c19e23bfdc..d161f44861 100644 --- a/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java +++ b/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java @@ -42,14 +42,19 @@ public class GapicServiceConfig { private final List methodConfigs; private final Map methodConfigTable; + private final Map batchingSettingsTable; private GapicServiceConfig( - List methodConfigs, Map methodConfigTable) { + List methodConfigs, + Map methodConfigTable, + Map batchingSettingsTable) { this.methodConfigs = methodConfigs; this.methodConfigTable = methodConfigTable; + this.batchingSettingsTable = batchingSettingsTable; } - public static GapicServiceConfig create(ServiceConfig serviceConfig) { + public static GapicServiceConfig create( + ServiceConfig serviceConfig, Optional> batchingSettingsOpt) { // Keep this processing logic out of the constructor. Map methodConfigTable = new HashMap<>(); List methodConfigs = serviceConfig.getMethodConfigList(); @@ -60,7 +65,21 @@ public static GapicServiceConfig create(ServiceConfig serviceConfig) { } } - return new GapicServiceConfig(methodConfigs, methodConfigTable); + Map batchingSettingsTable = new HashMap<>(); + if (batchingSettingsOpt.isPresent()) { + for (GapicBatchingSettings batchingSetting : batchingSettingsOpt.get()) { + batchingSettingsTable.put( + MethodConfig.Name.newBuilder() + .setService( + String.format( + "%s.%s", batchingSetting.protoPakkage(), batchingSetting.serviceName())) + .setMethod(batchingSetting.methodName()) + .build(), + batchingSetting); + } + } + + return new GapicServiceConfig(methodConfigs, methodConfigTable, batchingSettingsTable); } public Map getAllGapicRetrySettings(Service service) { @@ -68,28 +87,7 @@ public Map getAllGapicRetrySettings(Service service) .collect( Collectors.toMap( m -> getRetryParamsName(service, m), - m -> { - GapicRetrySettings.Kind kind = GapicRetrySettings.Kind.FULL; - Optional retryPolicyIndexOpt = retryPolicyIndexLookup(service, m); - if (!retryPolicyIndexOpt.isPresent()) { - kind = GapicRetrySettings.Kind.NONE; - } else { - MethodConfig methodConfig = methodConfigs.get(retryPolicyIndexOpt.get()); - if (!methodConfig.hasTimeout() && !methodConfig.hasRetryPolicy()) { - kind = GapicRetrySettings.Kind.NONE; - } else { - kind = - methodConfig.hasRetryPolicy() - ? GapicRetrySettings.Kind.FULL - : GapicRetrySettings.Kind.NO_RETRY; - } - } - return GapicRetrySettings.builder() - .setTimeout(timeoutLookup(service, m)) - .setRetryPolicy(retryPolicyLookup(service, m)) - .setKind(kind) - .build(); - }, + m -> toGapicRetrySettings(service, m), (r1, r2) -> r2, LinkedHashMap::new)); } @@ -120,6 +118,40 @@ public String getRetryParamsName(Service service, Method method) { return NO_RETRY_PARAMS_NAME; } + public boolean hasBatchingSetting(Service service, Method method) { + return batchingSettingsTable.containsKey(toName(service, method)); + } + + public Optional getBatchingSetting(Service service, Method method) { + return hasBatchingSetting(service, method) + ? Optional.of(batchingSettingsTable.get(toName(service, method))) + : Optional.empty(); + } + + private GapicRetrySettings toGapicRetrySettings(Service service, Method method) { + GapicRetrySettings.Kind kind = GapicRetrySettings.Kind.FULL; + Optional retryPolicyIndexOpt = retryPolicyIndexLookup(service, method); + if (!retryPolicyIndexOpt.isPresent()) { + kind = GapicRetrySettings.Kind.NONE; + } else { + MethodConfig methodConfig = methodConfigs.get(retryPolicyIndexOpt.get()); + if (!methodConfig.hasTimeout() && !methodConfig.hasRetryPolicy()) { + kind = GapicRetrySettings.Kind.NONE; + } else { + kind = + methodConfig.hasRetryPolicy() + ? GapicRetrySettings.Kind.FULL + : GapicRetrySettings.Kind.NO_RETRY; + } + } + + return GapicRetrySettings.builder() + .setTimeout(timeoutLookup(service, method)) + .setRetryPolicy(retryPolicyLookup(service, method)) + .setKind(kind) + .build(); + } + private List retryCodesLookup(Service service, Method method) { RetryPolicy retryPolicy = retryPolicyLookup(service, method); if (retryPolicy.equals(EMPTY_RETRY_POLICY)) { 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 ce55d25750..71ac168343 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 @@ -45,7 +45,14 @@ public class BatchingSettingsConfigParser { private static String YAML_KEY_BATCHING_FLOW_CONTROL_LIMIT_EXCEEDED_BEHAVIOR = "flow_control_limit_exceeded_behavior"; - public static Optional> parse(String gapicYamlConfigFilePath) { + public static Optional> parse( + Optional gapicYamlConfigFilePathOpt) { + return gapicYamlConfigFilePathOpt.isPresent() + ? parse(gapicYamlConfigFilePathOpt.get()) + : Optional.empty(); + } + + static Optional> parse(String gapicYamlConfigFilePath) { if (Strings.isNullOrEmpty(gapicYamlConfigFilePath) || !(new File(gapicYamlConfigFilePath)).exists()) { return Optional.empty(); 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 42c64e6d9f..7a6721710e 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 @@ -20,6 +20,7 @@ import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.VaporReference; import com.google.api.generator.gapic.model.Field; +import com.google.api.generator.gapic.model.GapicBatchingSettings; import com.google.api.generator.gapic.model.GapicContext; import com.google.api.generator.gapic.model.GapicServiceConfig; import com.google.api.generator.gapic.model.LongrunningOperation; @@ -70,15 +71,15 @@ public GapicParserException(String errorMessage) { } public static GapicContext parse(CodeGeneratorRequest request) { - Optional serviceConfigPathOpt = PluginArgumentParser.parseJsonConfigPath(request); - String serviceConfigPath = serviceConfigPathOpt.isPresent() ? serviceConfigPathOpt.get() : null; - Optional serviceConfigOpt = ServiceConfigParser.parse(serviceConfigPath); - - // TODO(miraleung): Actually pars the yaml file. Optional gapicYamlConfigPathOpt = PluginArgumentParser.parseGapicYamlConfigPath(request); - String gapicYamlConfigPath = - gapicYamlConfigPathOpt.isPresent() ? gapicYamlConfigPathOpt.get() : null; + Optional> batchingSettingsOpt = + BatchingSettingsConfigParser.parse(gapicYamlConfigPathOpt); + + Optional serviceConfigPathOpt = PluginArgumentParser.parseJsonConfigPath(request); + String serviceConfigPath = serviceConfigPathOpt.isPresent() ? serviceConfigPathOpt.get() : null; + Optional serviceConfigOpt = + ServiceConfigParser.parse(serviceConfigPath, batchingSettingsOpt); // Keep message and resource name parsing separate for cleaner logic. // While this takes an extra pass through the protobufs, the extra time is relatively trivial diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java index 24bbe84716..dcab2c4ec3 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java @@ -14,6 +14,7 @@ package com.google.api.generator.gapic.protoparser; +import com.google.api.generator.gapic.model.GapicBatchingSettings; import com.google.api.generator.gapic.model.GapicServiceConfig; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; @@ -22,15 +23,17 @@ import java.io.FileNotFoundException; import java.io.FileReader; import java.io.IOException; +import java.util.List; import java.util.Optional; public class ServiceConfigParser { - public static Optional parse(String serviceConfigFilePath) { + public static Optional parse( + String serviceConfigFilePath, Optional> batchingSettingsOpt) { Optional rawConfigOpt = parseFile(serviceConfigFilePath); if (!rawConfigOpt.isPresent()) { return Optional.empty(); } - return Optional.of(GapicServiceConfig.create(rawConfigOpt.get())); + return Optional.of(GapicServiceConfig.create(rawConfigOpt.get(), batchingSettingsOpt)); } @VisibleForTesting 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 d4aae0abb3..3c4a14e5e4 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 @@ -81,7 +81,8 @@ public void paramDefinitionsBlock_noConfigsFound() { String jsonFilename = "retrying_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -117,7 +118,8 @@ public void paramDefinitionsBlock_basic() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -157,7 +159,8 @@ public void codesDefinitionsBlock_noConfigsFound() { String jsonFilename = "retrying_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -193,7 +196,8 @@ public void codesDefinitionsBlock_basic() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -232,7 +236,8 @@ public void simpleBuilderExpr_basic() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -313,7 +318,8 @@ public void lroBuilderExpr() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); 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 75cb1b3275..4050efe984 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 @@ -62,7 +62,8 @@ public void generateServiceClasses() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional configOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional configOpt = + ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); assertTrue(configOpt.isPresent()); GapicServiceConfig config = configOpt.get(); 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 dbeb6c8e92..6b6618a2dd 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 @@ -16,6 +16,7 @@ import static com.google.common.truth.Truth.assertThat; import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertTrue; import com.google.api.generator.gapic.protoparser.Parser; @@ -28,6 +29,7 @@ import io.grpc.serviceconfig.MethodConfig; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -48,7 +50,9 @@ public void serviceConfig_noConfigsFound() { String jsonFilename = "retrying_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional> batchingSettingsOpt = Optional.empty(); + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -77,7 +81,9 @@ public void serviceConfig_basic() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); + Optional> batchingSettingsOpt = Optional.empty(); + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -124,6 +130,78 @@ public void serviceConfig_basic() { assertEquals(GapicServiceConfig.EMPTY_RETRY_CODES, retryCodes.get(retryCodeName)); } + @Test + public void serviceConfig_withBatchingSettings() { + FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor(); + ServiceDescriptor echoServiceDescriptor = echoFileDescriptor.getServices().get(0); + Service service = parseService(echoFileDescriptor); + + String jsonFilename = "showcase_grpc_service_config.json"; + Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); + + GapicBatchingSettings origBatchingSetting = + GapicBatchingSettings.builder() + .setProtoPakkage("google.showcase.v1beta1") + .setServiceName("Echo") + .setMethodName("Echo") + .setElementCountThreshold(1000) + .setRequestByteThreshold(2000) + .setDelayThresholdMillis(3000) + .build(); + Optional> batchingSettingsOpt = + Optional.of(Arrays.asList(origBatchingSetting)); + + Optional serviceConfigOpt = + ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); + assertTrue(serviceConfigOpt.isPresent()); + GapicServiceConfig serviceConfig = serviceConfigOpt.get(); + + Map retrySettings = serviceConfig.getAllGapicRetrySettings(service); + assertEquals(2, retrySettings.size()); + Map> retryCodes = serviceConfig.getAllRetryCodes(service); + assertEquals(2, retryCodes.size()); + + // Echo method has an explicitly-defined setting. + Method method = findMethod(service, "Echo"); + assertThat(method).isNotNull(); + + // No change to the retry settings. + String retryParamsName = serviceConfig.getRetryParamsName(service, method); + assertEquals("retry_policy_1_params", retryParamsName); + GapicRetrySettings settings = retrySettings.get(retryParamsName); + assertThat(settings).isNotNull(); + assertEquals(10, settings.timeout().getSeconds()); + assertEquals(GapicRetrySettings.Kind.FULL, settings.kind()); + + // No changge to the retry codes. + String retryCodeName = serviceConfig.getRetryCodeName(service, method); + assertEquals("retry_policy_1_codes", retryCodeName); + List retryCode = retryCodes.get(retryCodeName); + assertThat(retryCode).containsExactly(Code.UNAVAILABLE, Code.UNKNOWN); + + // Check batching settings. + assertTrue(serviceConfig.hasBatchingSetting(service, method)); + Optional batchingSettingOpt = + serviceConfig.getBatchingSetting(service, method); + assertTrue(batchingSettingOpt.isPresent()); + GapicBatchingSettings batchingSetting = batchingSettingOpt.get(); + assertEquals( + origBatchingSetting.elementCountThreshold(), batchingSetting.elementCountThreshold()); + assertEquals( + origBatchingSetting.requestByteThreshold(), batchingSetting.requestByteThreshold()); + assertEquals( + origBatchingSetting.delayThresholdMillis(), batchingSetting.delayThresholdMillis()); + + // Chat method defaults to the service-defined setting. + method = findMethod(service, "Chat"); + assertThat(method).isNotNull(); + retryParamsName = serviceConfig.getRetryParamsName(service, method); + assertEquals("no_retry_0_params", retryParamsName); + retryCodeName = serviceConfig.getRetryCodeName(service, method); + assertEquals("no_retry_0_codes", retryCodeName); + assertFalse(serviceConfig.hasBatchingSetting(service, method)); + } + private static Service parseService(FileDescriptor fileDescriptor) { Map messageTypes = Parser.parseMessages(fileDescriptor); Map resourceNames = Parser.parseResourceNames(fileDescriptor);