Skip to content

Commit 4a058ab

Browse files
miraleungmiraleung
authored
[ggj][codegen] fix: add LRO async variants, refactor ServiceClient methodgen (#354)
* feat: add streaming test variants to ServiceClientTest * fix: add LRO async variants, refactor ServiceClient codegen
1 parent 9b1eccf commit 4a058ab

5 files changed

Lines changed: 134 additions & 73 deletions

File tree

‎src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java‎

Lines changed: 51 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -462,12 +462,12 @@ private static List<MethodDefinition> createServiceMethods(
462462
Service service, Map<String, Message> messageTypes, Map<String, TypeNode> types) {
463463
List<MethodDefinition> javaMethods = new ArrayList<>();
464464
for (Method method : service.methods()) {
465-
if (method.stream().equals(Stream.NONE) && !method.hasLro()) {
465+
if (method.stream().equals(Stream.NONE)) {
466466
javaMethods.addAll(createMethodVariants(method, messageTypes, types));
467+
javaMethods.add(createMethodDefaultMethod(method, types));
467468
}
468469
if (method.hasLro()) {
469-
javaMethods.add(createLroAsyncMethod(service.name(), method, types));
470-
javaMethods.add(createLroCallable(service.name(), method, types));
470+
javaMethods.add(createLroCallableMethod(service.name(), method, types));
471471
}
472472
if (method.isPaged()) {
473473
javaMethods.add(createPagedCallableMethod(service.name(), method, types));
@@ -486,6 +486,18 @@ private static List<MethodDefinition> createMethodVariants(
486486
method.isPaged()
487487
? types.get(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, method.name()))
488488
: method.outputType();
489+
if (method.hasLro()) {
490+
LongrunningOperation lro = method.lro();
491+
methodOutputType =
492+
TypeNode.withReference(
493+
types
494+
.get("OperationFuture")
495+
.reference()
496+
.copyAndSetGenerics(
497+
Arrays.asList(
498+
lro.responseType().reference(), lro.metadataType().reference())));
499+
}
500+
489501
String methodInputTypeName = methodInputType.reference().name();
490502
Reference listRef = ConcreteReference.withClazz(List.class);
491503
Reference mapRef = ConcreteReference.withClazz(Map.class);
@@ -603,7 +615,7 @@ private static List<MethodDefinition> createMethodVariants(
603615
// Return expression.
604616
MethodInvocationExpr returnExpr =
605617
MethodInvocationExpr.builder()
606-
.setMethodName(methodName)
618+
.setMethodName(String.format(method.hasLro() ? "%sAsync" : "%s", methodName))
607619
.setArguments(Arrays.asList(requestVarExpr.toBuilder().setIsDecl(false).build()))
608620
.setReturnType(methodOutputType)
609621
.build();
@@ -615,14 +627,37 @@ private static List<MethodDefinition> createMethodVariants(
615627
.setScope(ScopeNode.PUBLIC)
616628
.setIsFinal(true)
617629
.setReturnType(methodOutputType)
618-
.setName(methodName)
630+
.setName(String.format(method.hasLro() ? "%sAsync" : "%s", methodName))
619631
.setArguments(arguments)
620632
.setBody(statements)
621633
.setReturnExpr(returnExpr)
622634
.build());
623635
}
624636

625-
// Finally, construct the method that accepts a request proto.
637+
return javaMethods;
638+
}
639+
640+
private static MethodDefinition createMethodDefaultMethod(
641+
Method method, Map<String, TypeNode> types) {
642+
String methodName = JavaStyle.toLowerCamelCase(method.name());
643+
TypeNode methodInputType = method.inputType();
644+
TypeNode methodOutputType =
645+
method.isPaged()
646+
? types.get(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, method.name()))
647+
: method.outputType();
648+
if (method.hasLro()) {
649+
LongrunningOperation lro = method.lro();
650+
methodOutputType =
651+
TypeNode.withReference(
652+
types
653+
.get("OperationFuture")
654+
.reference()
655+
.copyAndSetGenerics(
656+
Arrays.asList(
657+
lro.responseType().reference(), lro.metadataType().reference())));
658+
}
659+
660+
// Construct the method that accepts a request proto.
626661
VariableExpr requestArgVarExpr =
627662
VariableExpr.builder()
628663
.setVariable(Variable.builder().setName("request").setType(methodInputType).build())
@@ -632,79 +667,32 @@ private static List<MethodDefinition> createMethodVariants(
632667
method.isPaged()
633668
? String.format(PAGED_CALLABLE_NAME_PATTERN, methodName)
634669
: String.format(CALLABLE_NAME_PATTERN, methodName);
670+
if (method.hasLro()) {
671+
callableMethodName = String.format(OPERATION_CALLABLE_NAME_PATTERN, methodName);
672+
}
673+
635674
MethodInvocationExpr methodReturnExpr =
636675
MethodInvocationExpr.builder().setMethodName(callableMethodName).build();
637676
methodReturnExpr =
638677
MethodInvocationExpr.builder()
639-
.setMethodName("call")
678+
.setMethodName(method.hasLro() ? "futureCall" : "call")
640679
.setArguments(Arrays.asList(requestArgVarExpr.toBuilder().setIsDecl(false).build()))
641680
.setExprReferenceExpr(methodReturnExpr)
642681
.setReturnType(methodOutputType)
643682
.build();
644-
javaMethods.add(
645-
MethodDefinition.builder()
646-
.setHeaderCommentStatements(
647-
ServiceClientCommentComposer.createRpcMethodHeaderComment(method))
648-
.setScope(ScopeNode.PUBLIC)
649-
.setIsFinal(true)
650-
.setReturnType(methodOutputType)
651-
.setName(methodName)
652-
.setArguments(Arrays.asList(requestArgVarExpr))
653-
.setReturnExpr(methodReturnExpr)
654-
.build());
655-
656-
return javaMethods;
657-
}
658-
659-
private static MethodDefinition createLroAsyncMethod(
660-
String serviceName, Method method, Map<String, TypeNode> types) {
661-
// TODO(miraleung): Create variants as well.
662-
Preconditions.checkState(
663-
method.hasLro(), String.format("Method %s does not have an LRO", method.name()));
664-
String methodName = JavaStyle.toLowerCamelCase(method.name());
665-
TypeNode methodInputType = method.inputType();
666-
TypeNode methodOutputType = method.outputType();
667-
String methodInputTypeName = methodInputType.reference().name();
668-
LongrunningOperation lro = method.lro();
669-
670-
VariableExpr argumentExpr =
671-
VariableExpr.builder()
672-
.setVariable(Variable.builder().setName("request").setType(methodInputType).build())
673-
.setIsDecl(true)
674-
.build();
675-
676-
TypeNode returnType =
677-
TypeNode.withReference(
678-
types
679-
.get("OperationFuture")
680-
.reference()
681-
.copyAndSetGenerics(
682-
Arrays.asList(lro.responseType().reference(), lro.metadataType().reference())));
683-
MethodInvocationExpr returnExpr =
684-
MethodInvocationExpr.builder()
685-
.setMethodName(String.format("%sOperationCallable", methodName))
686-
.build();
687-
returnExpr =
688-
MethodInvocationExpr.builder()
689-
.setMethodName("futureCall")
690-
.setArguments(Arrays.asList(argumentExpr.toBuilder().setIsDecl(false).build()))
691-
.setExprReferenceExpr(returnExpr)
692-
.setReturnType(returnType)
693-
.build();
694-
695683
return MethodDefinition.builder()
696684
.setHeaderCommentStatements(
697685
ServiceClientCommentComposer.createRpcMethodHeaderComment(method))
698686
.setScope(ScopeNode.PUBLIC)
699687
.setIsFinal(true)
700-
.setReturnType(returnType)
701-
.setName(String.format("%sAsync", methodName))
702-
.setArguments(Arrays.asList(argumentExpr))
703-
.setReturnExpr(returnExpr)
688+
.setReturnType(methodOutputType)
689+
.setName(String.format(method.hasLro() ? "%sAsync" : "%s", methodName))
690+
.setArguments(Arrays.asList(requestArgVarExpr))
691+
.setReturnExpr(methodReturnExpr)
704692
.build();
705693
}
706694

707-
private static MethodDefinition createLroCallable(
695+
private static MethodDefinition createLroCallableMethod(
708696
String serviceName, Method method, Map<String, TypeNode> types) {
709697
return createCallableMethod(serviceName, method, types, CallableMethodKind.LRO);
710698
}

‎src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -609,9 +609,10 @@ private static MethodDefinition createRpcTestMethod(
609609
.build());
610610
} else {
611611
for (MethodArgument methodArg : methodSignature) {
612+
String methodArgName = JavaStyle.toLowerCamelCase(methodArg.name());
612613
VariableExpr varExpr =
613614
VariableExpr.withVariable(
614-
Variable.builder().setType(methodArg.type()).setName(methodArg.name()).build());
615+
Variable.builder().setType(methodArg.type()).setName(methodArgName).build());
615616
argExprs.add(varExpr);
616617
Expr valExpr = DefaultValueComposer.createDefaultValue(methodArg, resourceNames);
617618
methodExprs.add(
@@ -1385,9 +1386,10 @@ private static List<Statement> createRpcExceptionTestStatements(
13851386
.build());
13861387
} else {
13871388
for (MethodArgument methodArg : methodSignature) {
1389+
String methodArgName = JavaStyle.toLowerCamelCase(methodArg.name());
13881390
VariableExpr varExpr =
13891391
VariableExpr.withVariable(
1390-
Variable.builder().setType(methodArg.type()).setName(methodArg.name()).build());
1392+
Variable.builder().setType(methodArg.type()).setName(methodArgName).build());
13911393
argVarExprs.add(varExpr);
13921394
Expr valExpr = DefaultValueComposer.createDefaultValue(methodArg, resourceNames);
13931395
tryBodyExprs.add(

‎src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ public void generateServiceClasses() {
8383
+ "import com.google.common.util.concurrent.MoreExecutors;\n"
8484
+ "import com.google.longrunning.Operation;\n"
8585
+ "import com.google.longrunning.OperationsClient;\n"
86+
+ "import com.google.protobuf.Duration;\n"
87+
+ "import com.google.protobuf.Timestamp;\n"
8688
+ "import com.google.rpc.Status;\n"
8789
+ "import com.google.showcase.v1beta1.stub.EchoStub;\n"
8890
+ "import com.google.showcase.v1beta1.stub.EchoStubSettings;\n"
@@ -378,6 +380,32 @@ public void generateServiceClasses() {
378380
+ " /**\n"
379381
+ " * Sample code:\n"
380382
+ " *\n"
383+
+ " * @param ttl\n"
384+
+ " * @throws com.google.api.gax.rpc.ApiException if the remote call fails\n"
385+
+ " */\n"
386+
+ " public final OperationFuture<WaitResponse, WaitMetadata> waitAsync(Duration ttl)"
387+
+ " {\n"
388+
+ " WaitRequest request = WaitRequest.newBuilder().setTtl(ttl).build();\n"
389+
+ " return waitAsync(request);\n"
390+
+ " }\n"
391+
+ "\n"
392+
+ " // AUTO-GENERATED DOCUMENTATION AND METHOD.\n"
393+
+ " /**\n"
394+
+ " * Sample code:\n"
395+
+ " *\n"
396+
+ " * @param end_time\n"
397+
+ " * @throws com.google.api.gax.rpc.ApiException if the remote call fails\n"
398+
+ " */\n"
399+
+ " public final OperationFuture<WaitResponse, WaitMetadata> waitAsync(Timestamp"
400+
+ " endTime) {\n"
401+
+ " WaitRequest request = WaitRequest.newBuilder().setEndTime(endTime).build();\n"
402+
+ " return waitAsync(request);\n"
403+
+ " }\n"
404+
+ "\n"
405+
+ " // AUTO-GENERATED DOCUMENTATION AND METHOD.\n"
406+
+ " /**\n"
407+
+ " * Sample code:\n"
408+
+ " *\n"
381409
+ " * @param request The request object containing all of the parameters for the API"
382410
+ " call.\n"
383411
+ " * @throws com.google.api.gax.rpc.ApiException if the remote call fails\n"

‎src/test/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposerTest.java‎

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ public void generateServiceClasses() {
8383
+ "import com.google.longrunning.Operation;\n"
8484
+ "import com.google.protobuf.AbstractMessage;\n"
8585
+ "import com.google.protobuf.Any;\n"
86+
+ "import com.google.protobuf.Duration;\n"
87+
+ "import com.google.protobuf.Timestamp;\n"
8688
+ "import com.google.rpc.Status;\n"
8789
+ "import io.grpc.StatusRuntimeException;\n"
8890
+ "import java.io.IOException;\n"
@@ -637,16 +639,13 @@ public void generateServiceClasses() {
637639
+ " .setResponse(Any.pack(expectedResponse))\n"
638640
+ " .build();\n"
639641
+ " mockEcho.addResponse(resultOperation);\n"
640-
+ " WaitRequest request = WaitRequest.newBuilder().build();\n"
641-
+ " WaitResponse actualResponse = client.waitAsync(request).get();\n"
642+
+ " Duration ttl = Duration.newBuilder().build();\n"
643+
+ " WaitResponse actualResponse = client.waitAsync(ttl).get();\n"
642644
+ " Assert.assertEquals(expectedResponse, actualResponse);\n"
643645
+ " List<AbstractMessage> actualRequests = mockEcho.getRequests();\n"
644646
+ " Assert.assertEquals(1, actualRequests.size());\n"
645647
+ " WaitRequest actualRequest = ((WaitRequest) actualRequests.get(0));\n"
646-
+ " Assert.assertEquals(request.getEndTime(), actualRequest.getEndTime());\n"
647-
+ " Assert.assertEquals(request.getTtl(), actualRequest.getTtl());\n"
648-
+ " Assert.assertEquals(request.getError(), actualRequest.getError());\n"
649-
+ " Assert.assertEquals(request.getSuccess(), actualRequest.getSuccess());\n"
648+
+ " Assert.assertEquals(ttl, actualRequest.getTtl());\n"
650649
+ " Assert.assertTrue(\n"
651650
+ " channelProvider.isHeaderSent(\n"
652651
+ " ApiClientHeaderProvider.getDefaultApiClientHeaderKey(),\n"
@@ -659,8 +658,50 @@ public void generateServiceClasses() {
659658
+ " StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT);\n"
660659
+ " mockEcho.addException(exception);\n"
661660
+ " try {\n"
662-
+ " WaitRequest request = WaitRequest.newBuilder().build();\n"
663-
+ " client.waitAsync(request).get();\n"
661+
+ " Duration ttl = Duration.newBuilder().build();\n"
662+
+ " client.waitAsync(ttl).get();\n"
663+
+ " Assert.fail(\"No exception raised\");\n"
664+
+ " } catch (ExecutionException e) {\n"
665+
+ " Assert.assertEquals(InvalidArgumentException.class, e.getCause().getClass());\n"
666+
+ " InvalidArgumentException apiException = ((InvalidArgumentException)"
667+
+ " e.getCause());\n"
668+
+ " Assert.assertEquals(StatusCode.Code.INVALID_ARGUMENT,"
669+
+ " apiException.getStatusCode().getCode());\n"
670+
+ " }\n"
671+
+ " }\n"
672+
+ "\n"
673+
+ " @Test\n"
674+
+ " public void waitTest2() {\n"
675+
+ " WaitResponse expectedResponse =\n"
676+
+ " WaitResponse.newBuilder().setContent(\"content951530617\").build();\n"
677+
+ " Operation resultOperation =\n"
678+
+ " Operation.newBuilder()\n"
679+
+ " .setName(\"waitTest\")\n"
680+
+ " .setDone(true)\n"
681+
+ " .setResponse(Any.pack(expectedResponse))\n"
682+
+ " .build();\n"
683+
+ " mockEcho.addResponse(resultOperation);\n"
684+
+ " Timestamp endTime = Timestamp.newBuilder().build();\n"
685+
+ " WaitResponse actualResponse = client.waitAsync(endTime).get();\n"
686+
+ " Assert.assertEquals(expectedResponse, actualResponse);\n"
687+
+ " List<AbstractMessage> actualRequests = mockEcho.getRequests();\n"
688+
+ " Assert.assertEquals(1, actualRequests.size());\n"
689+
+ " WaitRequest actualRequest = ((WaitRequest) actualRequests.get(0));\n"
690+
+ " Assert.assertEquals(endTime, actualRequest.getEndTime());\n"
691+
+ " Assert.assertTrue(\n"
692+
+ " channelProvider.isHeaderSent(\n"
693+
+ " ApiClientHeaderProvider.getDefaultApiClientHeaderKey(),\n"
694+
+ " GaxGrpcProperties.getDefaultApiClientHeaderPattern()));\n"
695+
+ " }\n"
696+
+ "\n"
697+
+ " @Test\n"
698+
+ " public void waitExceptionTest2() throws Exception {\n"
699+
+ " StatusRuntimeException exception = new"
700+
+ " StatusRuntimeException(io.grpc.Status.INVALID_ARGUMENT);\n"
701+
+ " mockEcho.addException(exception);\n"
702+
+ " try {\n"
703+
+ " Timestamp endTime = Timestamp.newBuilder().build();\n"
704+
+ " client.waitAsync(endTime).get();\n"
664705
+ " Assert.fail(\"No exception raised\");\n"
665706
+ " } catch (ExecutionException e) {\n"
666707
+ " Assert.assertEquals(InvalidArgumentException.class, e.getCause().getClass());\n"

‎src/test/java/com/google/api/generator/gapic/testdata/echo.proto‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,8 @@ service Echo {
109109
response_type: "WaitResponse"
110110
metadata_type: "WaitMetadata"
111111
};
112+
option (google.api.method_signature) = "end_time";
113+
option (google.api.method_signature) = "ttl";
112114
}
113115

114116
// This method will block (wait) for the requested amount of time

0 commit comments

Comments
 (0)