Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ggj][codegen] fix: add LRO async variants, refactor ServiceClient methodgen #354

Merged
merged 3 commits into from
Oct 1, 2020
Merged

[ggj][codegen] fix: add LRO async variants, refactor ServiceClient methodgen #354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
539b5e6
feat: add streaming test variants to ServiceClientTest
miraleung Sep 28, 2020
6c6fc4b
fix: add LRO async variants, refactor ServiceClient codegen
miraleung Sep 28, 2020
6cfcb45
Merge branch 'master' into gp/g1
miraleung Oct 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 51 additions & 63 deletions src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,12 +462,12 @@ private static List<MethodDefinition> createServiceMethods(
Service service, Map<String, Message> messageTypes, Map<String, TypeNode> types) {
List<MethodDefinition> 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));
Expand All @@ -486,6 +486,18 @@ private static List<MethodDefinition> 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);
Expand Down Expand Up @@ -603,7 +615,7 @@ private static List<MethodDefinition> 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();
Expand All @@ -615,14 +627,37 @@ private static List<MethodDefinition> 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<String, TypeNode> 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())
Expand All @@ -632,79 +667,32 @@ private static List<MethodDefinition> 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<String, TypeNode> 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<String, TypeNode> types) {
return createCallableMethod(serviceName, method, types, CallableMethodKind.LRO);
}
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -1385,9 +1386,10 @@ private static List<Statement> 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(
Expand Down
28 changes: 28 additions & 0 deletions src/test/java/com/google/api/generator/gapic/composer/ServiceClientClassComposerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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<WaitResponse, WaitMetadata> 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<WaitResponse, WaitMetadata> 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"
Expand Down
57 changes: 49 additions & 8 deletions ...test/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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<AbstractMessage> 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"
Expand All @@ -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<AbstractMessage> 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"
Expand Down
2 changes: 2 additions & 0 deletions src/test/java/com/google/api/generator/gapic/testdata/echo.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down