From c71f061f96d8b418c8c1549cf65eb9fc45ee6396 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 25 Aug 2020 13:06:47 -0700 Subject: [PATCH 01/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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);