From 539b5e635835ba368fbe846ddf4ccecc14a8c4ac Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Mon, 28 Sep 2020 13:14:43 -0700 Subject: [PATCH 1/2] feat: add streaming test variants to ServiceClientTest --- .../ServiceClientTestClassComposer.java | 251 +++++++++++++++++- .../ServiceClientTestClassComposerTest.java | 106 ++++---- 2 files changed, 302 insertions(+), 55 deletions(-) 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 21ca7fa5fd..4fafe9b49b 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 @@ -416,7 +416,8 @@ private static List createTestMethods( Map messageTypes) { List javaMethods = new ArrayList<>(); for (Method method : service.methods()) { - if (method.methodSignatures().isEmpty()) { + // Ignore variants for streaming methods as well. + if (method.methodSignatures().isEmpty() || !method.stream().equals(Method.Stream.NONE)) { javaMethods.add( createRpcTestMethod( method, @@ -489,6 +490,10 @@ private static MethodDefinition createRpcTestMethod( Map classMemberVarExprs, Map resourceNames, Map messageTypes) { + if (!method.stream().equals(Method.Stream.NONE)) { + return createStreamingRpcTestMethod( + method, serviceName, classMemberVarExprs, resourceNames, messageTypes); + } // Construct the expected response. // TODO(miraleung): Paging here. TypeNode methodOutputType = method.hasLro() ? method.lro().responseType() : method.outputType(); @@ -890,6 +895,242 @@ private static MethodDefinition createRpcTestMethod( .build(); } + private static MethodDefinition createStreamingRpcTestMethod( + Method method, + String serviceName, + Map classMemberVarExprs, + Map resourceNames, + Map messageTypes) { + TypeNode methodOutputType = method.hasLro() ? method.lro().responseType() : method.outputType(); + List methodExprs = new ArrayList<>(); + VariableExpr expectedResponseVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(methodOutputType).setName("expectedResponse").build()); + Expr expectedResponseValExpr = null; + 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()); + } + methodExprs.add( + AssignmentExpr.builder() + .setVariableExpr(expectedResponseVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr(expectedResponseValExpr) + .build()); + if (method.hasLro()) { + VariableExpr resultOperationVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setType(STATIC_TYPES.get("Operation")) + .setName("resultOperation") + .build()); + methodExprs.add( + AssignmentExpr.builder() + .setVariableExpr(resultOperationVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr( + DefaultValueComposer.createSimpleOperationBuilderExpr( + String.format("%sTest", JavaStyle.toLowerCamelCase(method.name())), + expectedResponseVarExpr)) + .build()); + methodExprs.add( + MethodInvocationExpr.builder() + .setExprReferenceExpr(classMemberVarExprs.get(getMockServiceVarName(serviceName))) + .setMethodName("addResponse") + .setArguments(resultOperationVarExpr) + .build()); + } else { + methodExprs.add( + MethodInvocationExpr.builder() + .setExprReferenceExpr(classMemberVarExprs.get(getMockServiceVarName(serviceName))) + .setMethodName("addResponse") + .setArguments(expectedResponseVarExpr) + .build()); + } + // TODO(miraleung): Empty line here. + + // Construct the request or method arguments. + VariableExpr requestVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(method.inputType()).setName("request").build()); + Message requestMessage = messageTypes.get(method.inputType().reference().name()); + Preconditions.checkNotNull(requestMessage); + Expr valExpr = + DefaultValueComposer.createSimpleMessageBuilderExpr( + requestMessage, resourceNames, messageTypes); + methodExprs.add( + AssignmentExpr.builder() + .setVariableExpr(requestVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr(valExpr) + .build()); + + // Construct the mock stream observer. + VariableExpr responseObserverVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setType( + TypeNode.withReference( + STATIC_TYPES + .get("MockStreamObserver") + .reference() + .copyAndSetGenerics(Arrays.asList(method.outputType().reference())))) + .setName("responseObserver") + .build()); + + methodExprs.add( + AssignmentExpr.builder() + .setVariableExpr(responseObserverVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr( + NewObjectExpr.builder() + .setType(STATIC_TYPES.get("MockStreamObserver")) + .setIsGeneric(true) + .build()) + .build()); + + // Build the callable variable and assign it. + VariableExpr callableVarExpr = + VariableExpr.withVariable( + Variable.builder().setType(getCallableType(method)).setName("callable").build()); + Expr streamingCallExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(classMemberVarExprs.get("client")) + .setMethodName(String.format("%sCallable", JavaStyle.toLowerCamelCase(method.name()))) + .setReturnType(callableVarExpr.type()) + .build(); + methodExprs.add( + AssignmentExpr.builder() + .setVariableExpr(callableVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr(streamingCallExpr) + .build()); + + // Call the streaming-variant callable method. + if (method.stream().equals(Method.Stream.SERVER)) { + methodExprs.add( + MethodInvocationExpr.builder() + .setExprReferenceExpr(callableVarExpr) + .setMethodName("serverStreamingCall") + .setArguments(requestVarExpr, responseObserverVarExpr) + .build()); + } else { + VariableExpr requestObserverVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setType( + TypeNode.withReference( + STATIC_TYPES + .get("ApiStreamObserver") + .reference() + .copyAndSetGenerics(Arrays.asList(method.inputType().reference())))) + .setName("requestObserver") + .build()); + methodExprs.add( + AssignmentExpr.builder() + .setVariableExpr(requestObserverVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr( + MethodInvocationExpr.builder() + .setExprReferenceExpr(callableVarExpr) + .setMethodName(getCallableMethodName(method)) + .setArguments(requestVarExpr, responseObserverVarExpr) + .setReturnType(requestObserverVarExpr.type()) + .build()) + .build()); + + // TODO(miraleung): Empty line here. + methodExprs.add( + MethodInvocationExpr.builder() + .setExprReferenceExpr(requestObserverVarExpr) + .setMethodName("onNext") + .setArguments(requestVarExpr) + .build()); + methodExprs.add( + MethodInvocationExpr.builder() + .setExprReferenceExpr(requestObserverVarExpr) + .setMethodName("onCompleted") + .build()); + } + + // Check the actual responses. + VariableExpr actualResponsesVarExpr = + VariableExpr.withVariable( + Variable.builder() + .setType( + TypeNode.withReference( + ConcreteReference.builder() + .setClazz(List.class) + .setGenerics(Arrays.asList(method.outputType().reference())) + .build())) + .setName("actualResponses") + .build()); + + Expr getFutureResponseExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(responseObserverVarExpr) + .setMethodName("future") + .build(); + getFutureResponseExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(getFutureResponseExpr) + .setMethodName("get") + .setReturnType(actualResponsesVarExpr.type()) + .build(); + methodExprs.add( + AssignmentExpr.builder() + .setVariableExpr(actualResponsesVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr(getFutureResponseExpr) + .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(actualResponsesVarExpr) + .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 actualResponseExpr = + MethodInvocationExpr.builder() + .setExprReferenceExpr(actualResponsesVarExpr) + .setMethodName("get") + .setArguments(zeroExpr) + .build(); + + methodExprs.add( + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("Assert")) + .setMethodName("assertEquals") + .setArguments(expectedResponseVarExpr, actualResponseExpr) + .build()); + + String testMethodName = String.format("%sTest", JavaStyle.toLowerCamelCase(method.name())); + return MethodDefinition.builder() + .setAnnotations(Arrays.asList(TEST_ANNOTATION)) + .setScope(ScopeNode.PUBLIC) + .setReturnType(TypeNode.VOID) + .setName(testMethodName) + .setBody( + methodExprs.stream().map(e -> ExprStatement.withExpr(e)).collect(Collectors.toList())) + .build(); + } + private static MethodDefinition createRpcExceptionTestMethod( Method method, List methodSignature, @@ -938,11 +1179,11 @@ private static MethodDefinition createRpcExceptionTestMethod( methodBody.add(ExprStatement.withExpr(addExceptionExpr)); if (isStreaming) { methodBody.addAll( - createRpcExceptionTestStreamingStatements( + createStreamingRpcExceptionTestStatements( method, classMemberVarExprs, resourceNames, messageTypes)); } else { methodBody.addAll( - createRpcExceptionTestNonStreamingStatements( + createRpcExceptionTestStatements( method, methodSignature, classMemberVarExprs, resourceNames, messageTypes)); } @@ -956,7 +1197,7 @@ private static MethodDefinition createRpcExceptionTestMethod( .build(); } - private static List createRpcExceptionTestStreamingStatements( + private static List createStreamingRpcExceptionTestStatements( Method method, Map classMemberVarExprs, Map resourceNames, @@ -1118,7 +1359,7 @@ private static List createRpcExceptionTestStreamingStatements( return statements; } - private static List createRpcExceptionTestNonStreamingStatements( + private static List createRpcExceptionTestStatements( Method method, List methodSignature, Map classMemberVarExprs, 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 631fe0ad53..59ec4929e1 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 @@ -372,19 +372,16 @@ public void generateServiceClasses() { + " EchoResponse expectedResponse =\n" + " EchoResponse.newBuilder().setContent(\"content951530617\").build();\n" + " mockEcho.addResponse(expectedResponse);\n" - + " String content = \"content951530617\";\n" - + " Status error = Status.newBuilder().build();\n" - + " EchoResponse actualResponse = client.expand(content, error);\n" - + " Assert.assertEquals(expectedResponse, actualResponse);\n" - + " List actualRequests = mockEcho.getRequests();\n" - + " Assert.assertEquals(1, actualRequests.size());\n" - + " ExpandRequest actualRequest = ((ExpandRequest) actualRequests.get(0));\n" - + " Assert.assertEquals(content, actualRequest.getContent());\n" - + " Assert.assertEquals(error, actualRequest.getError());\n" - + " Assert.assertTrue(\n" - + " channelProvider.isHeaderSent(\n" - + " ApiClientHeaderProvider.getDefaultApiClientHeaderKey(),\n" - + " GaxGrpcProperties.getDefaultApiClientHeaderPattern()));\n" + + " ExpandRequest request =\n" + + " " + + " ExpandRequest.newBuilder().setContent(\"content951530617\").setInfo(\"info3237038\").build();\n" + + " MockStreamObserver responseObserver = new MockStreamObserver<>();\n" + + " ServerStreamingCallable callable =" + + " client.expandCallable();\n" + + " callable.serverStreamingCall(request, responseObserver);\n" + + " List actualResponses = responseObserver.future().get();\n" + + " Assert.assertEquals(1, actualResponses.size());\n" + + " Assert.assertEquals(expectedResponse, actualResponses.get(0));\n" + " }\n" + "\n" + " @Test\n" @@ -414,17 +411,24 @@ public void generateServiceClasses() { + " EchoResponse expectedResponse =\n" + " EchoResponse.newBuilder().setContent(\"content951530617\").build();\n" + " mockEcho.addResponse(expectedResponse);\n" - + " String content = \"content951530617\";\n" - + " EchoResponse actualResponse = client.collect(content);\n" - + " Assert.assertEquals(expectedResponse, actualResponse);\n" - + " List actualRequests = mockEcho.getRequests();\n" - + " Assert.assertEquals(1, actualRequests.size());\n" - + " EchoRequest actualRequest = ((EchoRequest) actualRequests.get(0));\n" - + " Assert.assertEquals(content, actualRequest.getContent());\n" - + " Assert.assertTrue(\n" - + " channelProvider.isHeaderSent(\n" - + " ApiClientHeaderProvider.getDefaultApiClientHeaderKey(),\n" - + " GaxGrpcProperties.getDefaultApiClientHeaderPattern()));\n" + + " EchoRequest request =\n" + + " EchoRequest.newBuilder()\n" + + " .setName(FoobarName.ofProjectFoobarName(\"[PROJECT]\"," + + " \"[FOOBAR]\").toString())\n" + + " .setParent(FoobarName.ofProjectFoobarName(\"[PROJECT]\"," + + " \"[FOOBAR]\").toString())\n" + + " .setFoobar(Foobar.newBuilder().build())\n" + + " .build();\n" + + " MockStreamObserver responseObserver = new MockStreamObserver<>();\n" + + " ClientStreamingCallable callable =" + + " client.collectCallable();\n" + + " ApiStreamObserver requestObserver =\n" + + " callable.clientStreamingCall(request, responseObserver);\n" + + " requestObserver.onNext(request);\n" + + " requestObserver.onCompleted();\n" + + " List actualResponses = responseObserver.future().get();\n" + + " Assert.assertEquals(1, actualResponses.size());\n" + + " Assert.assertEquals(expectedResponse, actualResponses.get(0));\n" + " }\n" + "\n" + " @Test\n" @@ -471,21 +475,16 @@ public void generateServiceClasses() { + " \"[FOOBAR]\").toString())\n" + " .setFoobar(Foobar.newBuilder().build())\n" + " .build();\n" - + " EchoResponse actualResponse = client.chat(request);\n" - + " Assert.assertEquals(expectedResponse, actualResponse);\n" - + " List actualRequests = mockEcho.getRequests();\n" - + " Assert.assertEquals(1, actualRequests.size());\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" - + " Assert.assertEquals(request.getError(), actualRequest.getError());\n" - + " Assert.assertEquals(request.getSeverity(), actualRequest.getSeverity());\n" - + " Assert.assertEquals(request.getFoobar(), actualRequest.getFoobar());\n" - + " Assert.assertTrue(\n" - + " channelProvider.isHeaderSent(\n" - + " ApiClientHeaderProvider.getDefaultApiClientHeaderKey(),\n" - + " GaxGrpcProperties.getDefaultApiClientHeaderPattern()));\n" + + " MockStreamObserver responseObserver = new MockStreamObserver<>();\n" + + " BidiStreamingCallable callable =" + + " client.chatCallable();\n" + + " ApiStreamObserver requestObserver =\n" + + " callable.bidiStreamingCall(request, responseObserver);\n" + + " requestObserver.onNext(request);\n" + + " requestObserver.onCompleted();\n" + + " List actualResponses = responseObserver.future().get();\n" + + " Assert.assertEquals(1, actualResponses.size());\n" + + " Assert.assertEquals(expectedResponse, actualResponses.get(0));\n" + " }\n" + "\n" + " @Test\n" @@ -524,17 +523,24 @@ public void generateServiceClasses() { + " EchoResponse expectedResponse =\n" + " EchoResponse.newBuilder().setContent(\"content951530617\").build();\n" + " mockEcho.addResponse(expectedResponse);\n" - + " String content = \"content951530617\";\n" - + " EchoResponse actualResponse = client.chatAgain(content);\n" - + " Assert.assertEquals(expectedResponse, actualResponse);\n" - + " List actualRequests = mockEcho.getRequests();\n" - + " Assert.assertEquals(1, actualRequests.size());\n" - + " EchoRequest actualRequest = ((EchoRequest) actualRequests.get(0));\n" - + " Assert.assertEquals(content, actualRequest.getContent());\n" - + " Assert.assertTrue(\n" - + " channelProvider.isHeaderSent(\n" - + " ApiClientHeaderProvider.getDefaultApiClientHeaderKey(),\n" - + " GaxGrpcProperties.getDefaultApiClientHeaderPattern()));\n" + + " EchoRequest request =\n" + + " EchoRequest.newBuilder()\n" + + " .setName(FoobarName.ofProjectFoobarName(\"[PROJECT]\"," + + " \"[FOOBAR]\").toString())\n" + + " .setParent(FoobarName.ofProjectFoobarName(\"[PROJECT]\"," + + " \"[FOOBAR]\").toString())\n" + + " .setFoobar(Foobar.newBuilder().build())\n" + + " .build();\n" + + " MockStreamObserver responseObserver = new MockStreamObserver<>();\n" + + " BidiStreamingCallable callable =" + + " client.chatAgainCallable();\n" + + " ApiStreamObserver requestObserver =\n" + + " callable.bidiStreamingCall(request, responseObserver);\n" + + " requestObserver.onNext(request);\n" + + " requestObserver.onCompleted();\n" + + " List actualResponses = responseObserver.future().get();\n" + + " Assert.assertEquals(1, actualResponses.size());\n" + + " Assert.assertEquals(expectedResponse, actualResponses.get(0));\n" + " }\n" + "\n" + " @Test\n" From 6c6fc4b6c59da245add7d626f28b982811e6c7fd Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Mon, 28 Sep 2020 15:21:32 -0700 Subject: [PATCH 2/2] fix: add LRO async variants, refactor ServiceClient codegen --- .../composer/ServiceClientClassComposer.java | 114 ++++++++---------- .../ServiceClientTestClassComposer.java | 6 +- .../ServiceClientClassComposerTest.java | 28 +++++ .../ServiceClientTestClassComposerTest.java | 57 +++++++-- .../api/generator/gapic/testdata/echo.proto | 2 + 5 files changed, 134 insertions(+), 73 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java index f9e99d7b18..177fffb162 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java @@ -462,12 +462,12 @@ private static List createServiceMethods( Service service, Map messageTypes, Map types) { List javaMethods = new ArrayList<>(); for (Method method : service.methods()) { - if (method.stream().equals(Stream.NONE) && !method.hasLro()) { + if (method.stream().equals(Stream.NONE)) { javaMethods.addAll(createMethodVariants(method, messageTypes, types)); + javaMethods.add(createMethodDefaultMethod(method, types)); } if (method.hasLro()) { - javaMethods.add(createLroAsyncMethod(service.name(), method, types)); - javaMethods.add(createLroCallable(service.name(), method, types)); + javaMethods.add(createLroCallableMethod(service.name(), method, types)); } if (method.isPaged()) { javaMethods.add(createPagedCallableMethod(service.name(), method, types)); @@ -486,6 +486,18 @@ private static List createMethodVariants( method.isPaged() ? types.get(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, method.name())) : method.outputType(); + if (method.hasLro()) { + LongrunningOperation lro = method.lro(); + methodOutputType = + TypeNode.withReference( + types + .get("OperationFuture") + .reference() + .copyAndSetGenerics( + Arrays.asList( + lro.responseType().reference(), lro.metadataType().reference()))); + } + String methodInputTypeName = methodInputType.reference().name(); Reference listRef = ConcreteReference.withClazz(List.class); Reference mapRef = ConcreteReference.withClazz(Map.class); @@ -603,7 +615,7 @@ private static List createMethodVariants( // Return expression. MethodInvocationExpr returnExpr = MethodInvocationExpr.builder() - .setMethodName(methodName) + .setMethodName(String.format(method.hasLro() ? "%sAsync" : "%s", methodName)) .setArguments(Arrays.asList(requestVarExpr.toBuilder().setIsDecl(false).build())) .setReturnType(methodOutputType) .build(); @@ -615,14 +627,37 @@ private static List createMethodVariants( .setScope(ScopeNode.PUBLIC) .setIsFinal(true) .setReturnType(methodOutputType) - .setName(methodName) + .setName(String.format(method.hasLro() ? "%sAsync" : "%s", methodName)) .setArguments(arguments) .setBody(statements) .setReturnExpr(returnExpr) .build()); } - // Finally, construct the method that accepts a request proto. + return javaMethods; + } + + private static MethodDefinition createMethodDefaultMethod( + Method method, Map types) { + String methodName = JavaStyle.toLowerCamelCase(method.name()); + TypeNode methodInputType = method.inputType(); + TypeNode methodOutputType = + method.isPaged() + ? types.get(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, method.name())) + : method.outputType(); + if (method.hasLro()) { + LongrunningOperation lro = method.lro(); + methodOutputType = + TypeNode.withReference( + types + .get("OperationFuture") + .reference() + .copyAndSetGenerics( + Arrays.asList( + lro.responseType().reference(), lro.metadataType().reference()))); + } + + // Construct the method that accepts a request proto. VariableExpr requestArgVarExpr = VariableExpr.builder() .setVariable(Variable.builder().setName("request").setType(methodInputType).build()) @@ -632,79 +667,32 @@ private static List createMethodVariants( method.isPaged() ? String.format(PAGED_CALLABLE_NAME_PATTERN, methodName) : String.format(CALLABLE_NAME_PATTERN, methodName); + if (method.hasLro()) { + callableMethodName = String.format(OPERATION_CALLABLE_NAME_PATTERN, methodName); + } + MethodInvocationExpr methodReturnExpr = MethodInvocationExpr.builder().setMethodName(callableMethodName).build(); methodReturnExpr = MethodInvocationExpr.builder() - .setMethodName("call") + .setMethodName(method.hasLro() ? "futureCall" : "call") .setArguments(Arrays.asList(requestArgVarExpr.toBuilder().setIsDecl(false).build())) .setExprReferenceExpr(methodReturnExpr) .setReturnType(methodOutputType) .build(); - javaMethods.add( - MethodDefinition.builder() - .setHeaderCommentStatements( - ServiceClientCommentComposer.createRpcMethodHeaderComment(method)) - .setScope(ScopeNode.PUBLIC) - .setIsFinal(true) - .setReturnType(methodOutputType) - .setName(methodName) - .setArguments(Arrays.asList(requestArgVarExpr)) - .setReturnExpr(methodReturnExpr) - .build()); - - return javaMethods; - } - - private static MethodDefinition createLroAsyncMethod( - String serviceName, Method method, Map types) { - // TODO(miraleung): Create variants as well. - Preconditions.checkState( - method.hasLro(), String.format("Method %s does not have an LRO", method.name())); - String methodName = JavaStyle.toLowerCamelCase(method.name()); - TypeNode methodInputType = method.inputType(); - TypeNode methodOutputType = method.outputType(); - String methodInputTypeName = methodInputType.reference().name(); - LongrunningOperation lro = method.lro(); - - VariableExpr argumentExpr = - VariableExpr.builder() - .setVariable(Variable.builder().setName("request").setType(methodInputType).build()) - .setIsDecl(true) - .build(); - - TypeNode returnType = - TypeNode.withReference( - types - .get("OperationFuture") - .reference() - .copyAndSetGenerics( - Arrays.asList(lro.responseType().reference(), lro.metadataType().reference()))); - MethodInvocationExpr returnExpr = - MethodInvocationExpr.builder() - .setMethodName(String.format("%sOperationCallable", methodName)) - .build(); - returnExpr = - MethodInvocationExpr.builder() - .setMethodName("futureCall") - .setArguments(Arrays.asList(argumentExpr.toBuilder().setIsDecl(false).build())) - .setExprReferenceExpr(returnExpr) - .setReturnType(returnType) - .build(); - return MethodDefinition.builder() .setHeaderCommentStatements( ServiceClientCommentComposer.createRpcMethodHeaderComment(method)) .setScope(ScopeNode.PUBLIC) .setIsFinal(true) - .setReturnType(returnType) - .setName(String.format("%sAsync", methodName)) - .setArguments(Arrays.asList(argumentExpr)) - .setReturnExpr(returnExpr) + .setReturnType(methodOutputType) + .setName(String.format(method.hasLro() ? "%sAsync" : "%s", methodName)) + .setArguments(Arrays.asList(requestArgVarExpr)) + .setReturnExpr(methodReturnExpr) .build(); } - private static MethodDefinition createLroCallable( + private static MethodDefinition createLroCallableMethod( String serviceName, Method method, Map types) { return createCallableMethod(serviceName, method, types, CallableMethodKind.LRO); } 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 4fafe9b49b..efd9732e3b 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 @@ -609,9 +609,10 @@ private static MethodDefinition createRpcTestMethod( .build()); } else { for (MethodArgument methodArg : methodSignature) { + String methodArgName = JavaStyle.toLowerCamelCase(methodArg.name()); VariableExpr varExpr = VariableExpr.withVariable( - Variable.builder().setType(methodArg.type()).setName(methodArg.name()).build()); + Variable.builder().setType(methodArg.type()).setName(methodArgName).build()); argExprs.add(varExpr); Expr valExpr = DefaultValueComposer.createDefaultValue(methodArg, resourceNames); methodExprs.add( @@ -1385,9 +1386,10 @@ private static List createRpcExceptionTestStatements( .build()); } else { for (MethodArgument methodArg : methodSignature) { + String methodArgName = JavaStyle.toLowerCamelCase(methodArg.name()); VariableExpr varExpr = VariableExpr.withVariable( - Variable.builder().setType(methodArg.type()).setName(methodArg.name()).build()); + Variable.builder().setType(methodArg.type()).setName(methodArgName).build()); argVarExprs.add(varExpr); Expr valExpr = DefaultValueComposer.createDefaultValue(methodArg, resourceNames); tryBodyExprs.add( diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java index 371b184823..0f210ac1a5 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java @@ -83,6 +83,8 @@ public void generateServiceClasses() { + "import com.google.common.util.concurrent.MoreExecutors;\n" + "import com.google.longrunning.Operation;\n" + "import com.google.longrunning.OperationsClient;\n" + + "import com.google.protobuf.Duration;\n" + + "import com.google.protobuf.Timestamp;\n" + "import com.google.rpc.Status;\n" + "import com.google.showcase.v1beta1.stub.EchoStub;\n" + "import com.google.showcase.v1beta1.stub.EchoStubSettings;\n" @@ -378,6 +380,32 @@ public void generateServiceClasses() { + " /**\n" + " * Sample code:\n" + " *\n" + + " * @param ttl\n" + + " * @throws com.google.api.gax.rpc.ApiException if the remote call fails\n" + + " */\n" + + " public final OperationFuture waitAsync(Duration ttl)" + + " {\n" + + " WaitRequest request = WaitRequest.newBuilder().setTtl(ttl).build();\n" + + " return waitAsync(request);\n" + + " }\n" + + "\n" + + " // AUTO-GENERATED DOCUMENTATION AND METHOD.\n" + + " /**\n" + + " * Sample code:\n" + + " *\n" + + " * @param end_time\n" + + " * @throws com.google.api.gax.rpc.ApiException if the remote call fails\n" + + " */\n" + + " public final OperationFuture waitAsync(Timestamp" + + " endTime) {\n" + + " WaitRequest request = WaitRequest.newBuilder().setEndTime(endTime).build();\n" + + " return waitAsync(request);\n" + + " }\n" + + "\n" + + " // AUTO-GENERATED DOCUMENTATION AND METHOD.\n" + + " /**\n" + + " * Sample code:\n" + + " *\n" + " * @param request The request object containing all of the parameters for the API" + " call.\n" + " * @throws com.google.api.gax.rpc.ApiException if the remote call fails\n" 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 59ec4929e1..973ba9393c 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 @@ -83,6 +83,8 @@ public void generateServiceClasses() { + "import com.google.longrunning.Operation;\n" + "import com.google.protobuf.AbstractMessage;\n" + "import com.google.protobuf.Any;\n" + + "import com.google.protobuf.Duration;\n" + + "import com.google.protobuf.Timestamp;\n" + "import com.google.rpc.Status;\n" + "import io.grpc.StatusRuntimeException;\n" + "import java.io.IOException;\n" @@ -637,16 +639,13 @@ public void generateServiceClasses() { + " .setResponse(Any.pack(expectedResponse))\n" + " .build();\n" + " mockEcho.addResponse(resultOperation);\n" - + " WaitRequest request = WaitRequest.newBuilder().build();\n" - + " WaitResponse actualResponse = client.waitAsync(request).get();\n" + + " Duration ttl = Duration.newBuilder().build();\n" + + " WaitResponse actualResponse = client.waitAsync(ttl).get();\n" + " Assert.assertEquals(expectedResponse, actualResponse);\n" + " List actualRequests = mockEcho.getRequests();\n" + " Assert.assertEquals(1, actualRequests.size());\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" - + " Assert.assertEquals(request.getSuccess(), actualRequest.getSuccess());\n" + + " Assert.assertEquals(ttl, actualRequest.getTtl());\n" + " Assert.assertTrue(\n" + " channelProvider.isHeaderSent(\n" + " ApiClientHeaderProvider.getDefaultApiClientHeaderKey(),\n" @@ -659,8 +658,50 @@ public void generateServiceClasses() { + " StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT);\n" + " mockEcho.addException(exception);\n" + " try {\n" - + " WaitRequest request = WaitRequest.newBuilder().build();\n" - + " client.waitAsync(request).get();\n" + + " Duration ttl = Duration.newBuilder().build();\n" + + " client.waitAsync(ttl).get();\n" + + " Assert.fail(\"No exception raised\");\n" + + " } catch (ExecutionException e) {\n" + + " Assert.assertEquals(InvalidArgumentException.class, e.getCause().getClass());\n" + + " InvalidArgumentException apiException = ((InvalidArgumentException)" + + " e.getCause());\n" + + " Assert.assertEquals(StatusCode.Code.INVALID_ARGUMENT," + + " apiException.getStatusCode().getCode());\n" + + " }\n" + + " }\n" + + "\n" + + " @Test\n" + + " public void waitTest2() {\n" + + " WaitResponse expectedResponse =\n" + + " WaitResponse.newBuilder().setContent(\"content951530617\").build();\n" + + " Operation resultOperation =\n" + + " Operation.newBuilder()\n" + + " .setName(\"waitTest\")\n" + + " .setDone(true)\n" + + " .setResponse(Any.pack(expectedResponse))\n" + + " .build();\n" + + " mockEcho.addResponse(resultOperation);\n" + + " Timestamp endTime = Timestamp.newBuilder().build();\n" + + " WaitResponse actualResponse = client.waitAsync(endTime).get();\n" + + " Assert.assertEquals(expectedResponse, actualResponse);\n" + + " List actualRequests = mockEcho.getRequests();\n" + + " Assert.assertEquals(1, actualRequests.size());\n" + + " WaitRequest actualRequest = ((WaitRequest) actualRequests.get(0));\n" + + " Assert.assertEquals(endTime, actualRequest.getEndTime());\n" + + " Assert.assertTrue(\n" + + " channelProvider.isHeaderSent(\n" + + " ApiClientHeaderProvider.getDefaultApiClientHeaderKey(),\n" + + " GaxGrpcProperties.getDefaultApiClientHeaderPattern()));\n" + + " }\n" + + "\n" + + " @Test\n" + + " public void waitExceptionTest2() throws Exception {\n" + + " StatusRuntimeException exception = new" + + " StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT);\n" + + " mockEcho.addException(exception);\n" + + " try {\n" + + " Timestamp endTime = Timestamp.newBuilder().build();\n" + + " client.waitAsync(endTime).get();\n" + " Assert.fail(\"No exception raised\");\n" + " } catch (ExecutionException e) {\n" + " Assert.assertEquals(InvalidArgumentException.class, e.getCause().getClass());\n" diff --git a/src/test/java/com/google/api/generator/gapic/testdata/echo.proto b/src/test/java/com/google/api/generator/gapic/testdata/echo.proto index e127d7ea40..19058ab680 100644 --- a/src/test/java/com/google/api/generator/gapic/testdata/echo.proto +++ b/src/test/java/com/google/api/generator/gapic/testdata/echo.proto @@ -109,6 +109,8 @@ service Echo { response_type: "WaitResponse" metadata_type: "WaitMetadata" }; + option (google.api.method_signature) = "end_time"; + option (google.api.method_signature) = "ttl"; } // This method will block (wait) for the requested amount of time