From 89fed885c17ef141584b43324347b64cc632e2c8 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 25 Sep 2020 20:00:45 -0700 Subject: [PATCH] [ggj][codegen][test] feat: ServiceClientTest.rpcTest support for paging (#352) * fix!: refactor field into MethodArgument, add enum/msg flags * feat: partial isAssignableFrom VaporRef support, enable full-name type usage * feat: support negative numeric literals * fix: separate resname tokenizing from class composer * feat: add per-type default value composer * feat: add ServiceClientTest.methodExceptionTests codegen * feat: add rpcExceptionTest for RPCs w/o overloads, support LRO * feat: ServiceClientTest.rpcExceptionTest support for LRO, streaming * fix: sorted method args * feat: add ServiceClientTest.rpcTest for unary and LRO methods * feat: ServiceClientTest.rpcTest support for paging * fix: merge master --- .../gapic/composer/DefaultValueComposer.java | 31 ++++ .../ServiceClientTestClassComposer.java | 166 +++++++++++++++--- .../ServiceClientTestClassComposerTest.java | 42 +++-- 3 files changed, 198 insertions(+), 41 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java b/src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java index 4a1bf86db3..2994fef37b 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/DefaultValueComposer.java @@ -282,4 +282,35 @@ static Expr createSimpleOperationBuilderExpr(String name, VariableExpr responseE .setReturnType(OPERATION_TYPE) .build(); } + + static Expr createSimplePagedResponse(TypeNode responseType, Expr responseElementVarExpr) { + Expr pagedResponseExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(responseType) + .setMethodName("newBuilder") + .build(); + pagedResponseExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(pagedResponseExpr) + .setMethodName("setNextPageToken") + .setArguments(ValueExpr.withValue(StringObjectValue.withValue(""))) + .build(); + pagedResponseExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(pagedResponseExpr) + .setMethodName("addAllResponses") + .setArguments( + MethodInvocationExpr.builder() + .setStaticReferenceType( + TypeNode.withReference(ConcreteReference.withClazz(Arrays.class))) + .setMethodName("asList") + .setArguments(responseElementVarExpr) + .build()) + .build(); + return MethodInvocationExpr.builder() + .setExprReferenceExpr(pagedResponseExpr) + .setMethodName("build") + .setReturnType(responseType) + .build(); + } } diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java index ee253b1a18..21ca7fa5fd 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java @@ -492,27 +492,60 @@ private static MethodDefinition createRpcTestMethod( // Construct the expected response. // TODO(miraleung): Paging here. TypeNode methodOutputType = method.hasLro() ? method.lro().responseType() : method.outputType(); + List methodExprs = new ArrayList<>(); + + TypeNode repeatedResponseType = null; + VariableExpr responsesElementVarExpr = null; + if (method.isPaged()) { + Message methodOutputMessage = messageTypes.get(method.outputType().reference().name()); + repeatedResponseType = findRepeatedPagedType(methodOutputMessage); + Preconditions.checkNotNull( + repeatedResponseType, + String.format( + "No repeated type found for paged method %s with output message type %s", + method.name(), methodOutputMessage.name())); + responsesElementVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(repeatedResponseType).setName("responsesElement").build()); + methodExprs.add( + AssignmentExpr.builder() + .setVariableExpr(responsesElementVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr( + DefaultValueComposer.createDefaultValue( + Field.builder() + .setType(repeatedResponseType) + .setName("responsesElement") + .setIsMessage(true) + .build())) + .build()); + } + VariableExpr expectedResponseVarExpr = VariableExpr.withVariable( Variable.builder().setType(methodOutputType).setName("expectedResponse").build()); Expr expectedResponseValExpr = null; - if (messageTypes.containsKey(methodOutputType.reference().name())) { + if (method.isPaged()) { expectedResponseValExpr = - DefaultValueComposer.createSimpleMessageBuilderExpr( - messageTypes.get(methodOutputType.reference().name()), resourceNames, messageTypes); + DefaultValueComposer.createSimplePagedResponse( + method.outputType(), responsesElementVarExpr); } else { - // Wrap this in a field so we don't have to split the helper into lots of different methods, - // or duplicate it for VariableExpr. - expectedResponseValExpr = - DefaultValueComposer.createDefaultValue( - Field.builder() - .setType(methodOutputType) - .setIsMessage(true) - .setName("expectedResponse") - .build()); + if (messageTypes.containsKey(methodOutputType.reference().name())) { + expectedResponseValExpr = + DefaultValueComposer.createSimpleMessageBuilderExpr( + messageTypes.get(methodOutputType.reference().name()), resourceNames, messageTypes); + } else { + // Wrap this in a field so we don't have to split the helper into lots of different methods, + // or duplicate it for VariableExpr. + expectedResponseValExpr = + DefaultValueComposer.createDefaultValue( + Field.builder() + .setType(methodOutputType) + .setIsMessage(true) + .setName("expectedResponse") + .build()); + } } - List methodExprs = new ArrayList<>(); methodExprs.add( AssignmentExpr.builder() .setVariableExpr(expectedResponseVarExpr.toBuilder().setIsDecl(true).build()) @@ -588,7 +621,10 @@ private static MethodDefinition createRpcTestMethod( // Call the RPC Java method. VariableExpr actualResponseVarExpr = VariableExpr.withVariable( - Variable.builder().setType(methodOutputType).setName("actualResponse").build()); + Variable.builder() + .setType(methodOutputType) + .setName(method.isPaged() ? "pagedListResponse" : "actualResponse") + .build()); Expr rpcJavaMethodInvocationExpr = MethodInvocationExpr.builder() .setExprReferenceExpr(classMemberVarExprs.get("client")) @@ -610,12 +646,88 @@ private static MethodDefinition createRpcTestMethod( .setVariableExpr(actualResponseVarExpr.toBuilder().setIsDecl(true).build()) .setValueExpr(rpcJavaMethodInvocationExpr) .build()); - methodExprs.add( - MethodInvocationExpr.builder() - .setStaticReferenceType(STATIC_TYPES.get("Assert")) - .setMethodName("assertEquals") - .setArguments(expectedResponseVarExpr, actualResponseVarExpr) - .build()); + + if (method.isPaged()) { + // Assign the resources variaqble. + VariableExpr resourcesVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setType( + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(List.class) + .setGenerics(Arrays.asList(repeatedResponseType.reference())) + .build())) + .setName("resources") + .build()); + Expr iterateAllExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(actualResponseVarExpr) + .setMethodName("iterateAll") + .build(); + Expr resourcesValExpr = + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("Lists")) + .setMethodName("newArrayList") + .setArguments(iterateAllExpr) + .setReturnType(resourcesVarExpr.type()) + .build(); + methodExprs.add( + AssignmentExpr.builder() + .setVariableExpr(resourcesVarExpr) + .setValueExpr(resourcesValExpr) + .build()); + + // Assert the size is equivalent. + methodExprs.add( + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("Assert")) + .setMethodName("assertEquals") + .setArguments( + ValueExpr.withValue( + PrimitiveValue.builder().setType(TypeNode.INT).setValue("1").build()), + MethodInvocationExpr.builder() + .setExprReferenceExpr(resourcesVarExpr) + .setMethodName("size") + .setReturnType(TypeNode.INT) + .build()) + .build()); + + // Assert the responses are equivalent. + Expr zeroExpr = + ValueExpr.withValue(PrimitiveValue.builder().setType(TypeNode.INT).setValue("0").build()); + Expr expectedPagedResponseExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(expectedResponseVarExpr) + .setMethodName("getResponsesList") + .build(); + expectedPagedResponseExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(expectedPagedResponseExpr) + .setMethodName("get") + .setArguments(zeroExpr) + .build(); + Expr actualPagedResponseExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(resourcesVarExpr) + .setMethodName("get") + .setArguments(zeroExpr) + .build(); + + methodExprs.add( + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("Assert")) + .setMethodName("assertEquals") + .setArguments(expectedPagedResponseExpr, actualPagedResponseExpr) + .build()); + } else { + methodExprs.add( + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("Assert")) + .setMethodName("assertEquals") + .setArguments(expectedResponseVarExpr, actualResponseVarExpr) + .build()); + } // TODO(miraleung): Empty line here. // Construct the request checker logic. @@ -658,7 +770,7 @@ private static MethodDefinition createRpcTestMethod( VariableExpr actualRequestVarExpr = VariableExpr.withVariable( - Variable.builder().setType(methodOutputType).setName("actualRequest").build()); + Variable.builder().setType(method.inputType()).setName("actualRequest").build()); Expr getFirstRequestExpr = MethodInvocationExpr.builder() .setExprReferenceExpr(actualRequestsVarExpr) @@ -669,7 +781,7 @@ private static MethodDefinition createRpcTestMethod( .setReturnType(STATIC_TYPES.get("AbstractMessage")) .build(); getFirstRequestExpr = - CastExpr.builder().setType(methodOutputType).setExpr(getFirstRequestExpr).build(); + CastExpr.builder().setType(method.inputType()).setExpr(getFirstRequestExpr).build(); methodExprs.add( AssignmentExpr.builder() .setVariableExpr(actualRequestVarExpr.toBuilder().setIsDecl(true).build()) @@ -1437,6 +1549,16 @@ private static TypeNode getCallableType(Method protoMethod) { ConcreteReference.builder().setClazz(callableClazz).setGenerics(generics).build()); } + private static TypeNode findRepeatedPagedType(Message message) { + for (Field field : message.fields()) { + if (field.isRepeated() && !field.isMap()) { + Reference repeatedGenericRef = field.type().reference().generics().get(0); + return TypeNode.withReference(repeatedGenericRef); + } + } + return null; + } + private static String getCallableMethodName(Method protoMethod) { Preconditions.checkState( !protoMethod.stream().equals(Method.Stream.NONE), diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposerTest.java index e9ee79f550..631fe0ad53 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposerTest.java @@ -79,13 +79,13 @@ public void generateServiceClasses() { + "import com.google.api.gax.rpc.ServerStreamingCallable;\n" + "import com.google.api.gax.rpc.StatusCode;\n" + "import com.google.api.resourcenames.ResourceName;\n" + + "import com.google.common.collect.Lists;\n" + "import com.google.longrunning.Operation;\n" + "import com.google.protobuf.AbstractMessage;\n" + "import com.google.protobuf.Any;\n" + "import com.google.rpc.Status;\n" + "import io.grpc.StatusRuntimeException;\n" + "import java.io.IOException;\n" - + "import java.util.ArrayList;\n" + "import java.util.Arrays;\n" + "import java.util.List;\n" + "import java.util.UUID;\n" @@ -148,7 +148,7 @@ public void generateServiceClasses() { + " Assert.assertEquals(expectedResponse, actualResponse);\n" + " List actualRequests = mockEcho.getRequests();\n" + " Assert.assertEquals(1, actualRequests.size());\n" - + " EchoResponse actualRequest = ((EchoResponse) actualRequests.get(0));\n" + + " EchoRequest actualRequest = ((EchoRequest) actualRequests.get(0));\n" + " Assert.assertEquals(parent.toString(), actualRequest.getParent());\n" + " Assert.assertTrue(\n" + " channelProvider.isHeaderSent(\n" @@ -181,7 +181,7 @@ public void generateServiceClasses() { + " Assert.assertEquals(expectedResponse, actualResponse);\n" + " List actualRequests = mockEcho.getRequests();\n" + " Assert.assertEquals(1, actualRequests.size());\n" - + " EchoResponse actualRequest = ((EchoResponse) actualRequests.get(0));\n" + + " EchoRequest actualRequest = ((EchoRequest) actualRequests.get(0));\n" + " Assert.assertEquals(error, actualRequest.getError());\n" + " Assert.assertTrue(\n" + " channelProvider.isHeaderSent(\n" @@ -213,7 +213,7 @@ public void generateServiceClasses() { + " Assert.assertEquals(expectedResponse, actualResponse);\n" + " List actualRequests = mockEcho.getRequests();\n" + " Assert.assertEquals(1, actualRequests.size());\n" - + " EchoResponse actualRequest = ((EchoResponse) actualRequests.get(0));\n" + + " EchoRequest actualRequest = ((EchoRequest) actualRequests.get(0));\n" + " Assert.assertEquals(name.toString(), actualRequest.getName());\n" + " Assert.assertTrue(\n" + " channelProvider.isHeaderSent(\n" @@ -246,7 +246,7 @@ public void generateServiceClasses() { + " Assert.assertEquals(expectedResponse, actualResponse);\n" + " List actualRequests = mockEcho.getRequests();\n" + " Assert.assertEquals(1, actualRequests.size());\n" - + " EchoResponse actualRequest = ((EchoResponse) actualRequests.get(0));\n" + + " EchoRequest actualRequest = ((EchoRequest) actualRequests.get(0));\n" + " Assert.assertEquals(content, actualRequest.getContent());\n" + " Assert.assertTrue(\n" + " channelProvider.isHeaderSent(\n" @@ -278,7 +278,7 @@ public void generateServiceClasses() { + " Assert.assertEquals(expectedResponse, actualResponse);\n" + " List actualRequests = mockEcho.getRequests();\n" + " Assert.assertEquals(1, actualRequests.size());\n" - + " EchoResponse actualRequest = ((EchoResponse) actualRequests.get(0));\n" + + " EchoRequest actualRequest = ((EchoRequest) actualRequests.get(0));\n" + " Assert.assertEquals(name, actualRequest.getName());\n" + " Assert.assertTrue(\n" + " channelProvider.isHeaderSent(\n" @@ -310,7 +310,7 @@ public void generateServiceClasses() { + " Assert.assertEquals(expectedResponse, actualResponse);\n" + " List actualRequests = mockEcho.getRequests();\n" + " Assert.assertEquals(1, actualRequests.size());\n" - + " EchoResponse actualRequest = ((EchoResponse) actualRequests.get(0));\n" + + " EchoRequest actualRequest = ((EchoRequest) actualRequests.get(0));\n" + " Assert.assertEquals(parent, actualRequest.getParent());\n" + " Assert.assertTrue(\n" + " channelProvider.isHeaderSent(\n" @@ -343,7 +343,7 @@ public void generateServiceClasses() { + " Assert.assertEquals(expectedResponse, actualResponse);\n" + " List actualRequests = mockEcho.getRequests();\n" + " Assert.assertEquals(1, actualRequests.size());\n" - + " EchoResponse actualRequest = ((EchoResponse) actualRequests.get(0));\n" + + " EchoRequest actualRequest = ((EchoRequest) actualRequests.get(0));\n" + " Assert.assertEquals(content, actualRequest.getContent());\n" + " Assert.assertEquals(severity, actualRequest.getSeverity());\n" + " Assert.assertTrue(\n" @@ -378,7 +378,7 @@ public void generateServiceClasses() { + " Assert.assertEquals(expectedResponse, actualResponse);\n" + " List actualRequests = mockEcho.getRequests();\n" + " Assert.assertEquals(1, actualRequests.size());\n" - + " EchoResponse actualRequest = ((EchoResponse) actualRequests.get(0));\n" + + " ExpandRequest actualRequest = ((ExpandRequest) actualRequests.get(0));\n" + " Assert.assertEquals(content, actualRequest.getContent());\n" + " Assert.assertEquals(error, actualRequest.getError());\n" + " Assert.assertTrue(\n" @@ -419,7 +419,7 @@ public void generateServiceClasses() { + " Assert.assertEquals(expectedResponse, actualResponse);\n" + " List actualRequests = mockEcho.getRequests();\n" + " Assert.assertEquals(1, actualRequests.size());\n" - + " EchoResponse actualRequest = ((EchoResponse) actualRequests.get(0));\n" + + " EchoRequest actualRequest = ((EchoRequest) actualRequests.get(0));\n" + " Assert.assertEquals(content, actualRequest.getContent());\n" + " Assert.assertTrue(\n" + " channelProvider.isHeaderSent(\n" @@ -475,7 +475,7 @@ public void generateServiceClasses() { + " Assert.assertEquals(expectedResponse, actualResponse);\n" + " List actualRequests = mockEcho.getRequests();\n" + " Assert.assertEquals(1, actualRequests.size());\n" - + " EchoResponse actualRequest = ((EchoResponse) actualRequests.get(0));\n" + + " EchoRequest actualRequest = ((EchoRequest) actualRequests.get(0));\n" + " Assert.assertEquals(request.getName(), actualRequest.getName());\n" + " Assert.assertEquals(request.getParent(), actualRequest.getParent());\n" + " Assert.assertEquals(request.getContent(), actualRequest.getContent());\n" @@ -529,7 +529,7 @@ public void generateServiceClasses() { + " Assert.assertEquals(expectedResponse, actualResponse);\n" + " List actualRequests = mockEcho.getRequests();\n" + " Assert.assertEquals(1, actualRequests.size());\n" - + " EchoResponse actualRequest = ((EchoResponse) actualRequests.get(0));\n" + + " EchoRequest actualRequest = ((EchoRequest) actualRequests.get(0));\n" + " Assert.assertEquals(content, actualRequest.getContent());\n" + " Assert.assertTrue(\n" + " channelProvider.isHeaderSent(\n" @@ -570,10 +570,11 @@ public void generateServiceClasses() { + "\n" + " @Test\n" + " public void pagedExpandTest() {\n" + + " EchoResponse responsesElement = EchoResponse.newBuilder().build();\n" + " PagedExpandResponse expectedResponse =\n" + " PagedExpandResponse.newBuilder()\n" - + " .addAllResponses(new ArrayList<>())\n" - + " .setNextPageToken(\"next_page_token-1530815211\")\n" + + " .setNextPageToken(\"\")\n" + + " .addAllResponses(Arrays.asList(responsesElement))\n" + " .build();\n" + " mockEcho.addResponse(expectedResponse);\n" + " PagedExpandRequest request =\n" @@ -582,11 +583,14 @@ public void generateServiceClasses() { + " .setPageSize(883849137)\n" + " .setPageToken(\"page_token1630607433\")\n" + " .build();\n" - + " PagedExpandResponse actualResponse = client.pagedExpand(request);\n" - + " Assert.assertEquals(expectedResponse, actualResponse);\n" + + " PagedExpandResponse pagedListResponse = client.pagedExpand(request);\n" + + " resources = Lists.newArrayList(pagedListResponse.iterateAll());\n" + + " Assert.assertEquals(1, resources.size());\n" + + " Assert.assertEquals(expectedResponse.getResponsesList().get(0)," + + " resources.get(0));\n" + " List actualRequests = mockEcho.getRequests();\n" + " Assert.assertEquals(1, actualRequests.size());\n" - + " PagedExpandResponse actualRequest = ((PagedExpandResponse)" + + " PagedExpandRequest actualRequest = ((PagedExpandRequest)" + " actualRequests.get(0));\n" + " Assert.assertEquals(request.getContent(), actualRequest.getContent());\n" + " Assert.assertEquals(request.getPageSize(), actualRequest.getPageSize());\n" @@ -632,7 +636,7 @@ public void generateServiceClasses() { + " Assert.assertEquals(expectedResponse, actualResponse);\n" + " List actualRequests = mockEcho.getRequests();\n" + " Assert.assertEquals(1, actualRequests.size());\n" - + " WaitResponse actualRequest = ((WaitResponse) actualRequests.get(0));\n" + + " WaitRequest actualRequest = ((WaitRequest) actualRequests.get(0));\n" + " Assert.assertEquals(request.getEndTime(), actualRequest.getEndTime());\n" + " Assert.assertEquals(request.getTtl(), actualRequest.getTtl());\n" + " Assert.assertEquals(request.getError(), actualRequest.getError());\n" @@ -671,7 +675,7 @@ public void generateServiceClasses() { + " Assert.assertEquals(expectedResponse, actualResponse);\n" + " List actualRequests = mockEcho.getRequests();\n" + " Assert.assertEquals(1, actualRequests.size());\n" - + " BlockResponse actualRequest = ((BlockResponse) actualRequests.get(0));\n" + + " BlockRequest actualRequest = ((BlockRequest) actualRequests.get(0));\n" + " Assert.assertEquals(request.getResponseDelay()," + " actualRequest.getResponseDelay());\n" + " Assert.assertEquals(request.getError(), actualRequest.getError());\n"