Skip to content

Commit

Permalink
[ggj][codegen][test] fix: Use repeated field name for paged RPC unit …
Browse files Browse the repository at this point in the history
…tests (#405)

* fix: refactor requestBuilder into separate method in ServiceClientClassComposer

* feat: add varargs to AnonClass and ref setter methods

* feat: add HTTP annotation parsing/validation

* feat: Generate RequestParamsExtractor in GrpcServiceStub

* feat: add GrpcPublisherStub test to exercise HTTP subfields

* fix: add ByteString to DefaultValueComposer

* fix: Use repeated field name for paged RPC unit tests

* fix:  change getter too
  • Loading branch information
miraleung authored Oct 24, 2020
1 parent 5b9c53f commit 3cf3ba3
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,8 @@ static Expr createSimpleOperationBuilderExpr(String name, VariableExpr responseE
.build();
}

static Expr createSimplePagedResponse(TypeNode responseType, Expr responseElementVarExpr) {
static Expr createSimplePagedResponse(
TypeNode responseType, String repeatedFieldName, Expr responseElementVarExpr) {
Expr pagedResponseExpr =
MethodInvocationExpr.builder()
.setStaticReferenceType(responseType)
Expand All @@ -309,7 +310,7 @@ static Expr createSimplePagedResponse(TypeNode responseType, Expr responseElemen
pagedResponseExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(pagedResponseExpr)
.setMethodName("addAllResponses")
.setMethodName(String.format("addAll%s", JavaStyle.toUpperCamelCase(repeatedFieldName)))
.setArguments(
MethodInvocationExpr.builder()
.setStaticReferenceType(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@ public class ServiceClientTestClassComposer {
private static final String GRPC_TESTING_PACKAGE = "com.google.api.gax.grpc.testing";
private static final String MOCK_SERVICE_CLASS_NAME_PATTERN = "Mock%s";
private static final String MOCK_SERVICE_VAR_NAME_PATTERN = "mock%s";
private static final String PAGED_RESPONSE_TYPE_NAME_PATTERN = "%sPagedResponse";
private static final String SERVICE_CLIENT_CLASS_NAME_PATTERN = "%sClient";
private static final String SERVICE_HELPER_VAR_NAME = "mockServiceHelper";
private static final String SERVICE_SETTINGS_CLASS_NAME_PATTERN = "%sSettings";
private static final String STUB_SETTINGS_PATTERN = "%sSettings";
private static final String PAGED_RESPONSE_TYPE_NAME_PATTERN = "%sPagedResponse";

private static final ServiceClientTestClassComposer INSTANCE =
new ServiceClientTestClassComposer();
Expand Down Expand Up @@ -424,18 +424,18 @@ private static List<MethodDefinition> createTestMethods(
javaMethods.add(
createRpcTestMethod(
method,
service,
Collections.emptyList(),
0,
service.name(),
classMemberVarExprs,
resourceNames,
messageTypes));
javaMethods.add(
createRpcExceptionTestMethod(
method,
service,
Collections.emptyList(),
0,
service.name(),
classMemberVarExprs,
resourceNames,
messageTypes));
Expand All @@ -444,18 +444,18 @@ private static List<MethodDefinition> createTestMethods(
javaMethods.add(
createRpcTestMethod(
method,
service,
method.methodSignatures().get(i),
i,
service.name(),
classMemberVarExprs,
resourceNames,
messageTypes));
javaMethods.add(
createRpcExceptionTestMethod(
method,
service,
method.methodSignatures().get(i),
i,
service.name(),
classMemberVarExprs,
resourceNames,
messageTypes));
Expand All @@ -467,12 +467,14 @@ private static List<MethodDefinition> createTestMethods(

private static MethodDefinition createRpcTestMethod(
Method method,
Service service,
List<MethodArgument> methodSignature,
int variantIndex,
String serviceName,
Map<String, VariableExpr> classMemberVarExprs,
Map<String, ResourceName> resourceNames,
Map<String, Message> messageTypes) {
String serviceName = service.name();

if (!method.stream().equals(Method.Stream.NONE)) {
return createStreamingRpcTestMethod(
method, serviceName, classMemberVarExprs, resourceNames, messageTypes);
Expand Down Expand Up @@ -514,9 +516,17 @@ private static MethodDefinition createRpcTestMethod(
Variable.builder().setType(methodOutputType).setName("expectedResponse").build());
Expr expectedResponseValExpr = null;
if (method.isPaged()) {
Message methodOutputMessage = messageTypes.get(method.outputType().reference().name());
Field firstRepeatedField = methodOutputMessage.findAndUnwrapFirstRepeatedField();
Preconditions.checkNotNull(
firstRepeatedField,
String.format(
"Expected paged RPC %s to have a repeated field in the response %s but found none",
method.name(), methodOutputMessage.name()));

expectedResponseValExpr =
DefaultValueComposer.createSimplePagedResponse(
method.outputType(), responsesElementVarExpr);
method.outputType(), firstRepeatedField.name(), responsesElementVarExpr);
} else {
if (messageTypes.containsKey(methodOutputType.reference().name())) {
expectedResponseValExpr =
Expand Down Expand Up @@ -619,7 +629,8 @@ private static MethodDefinition createRpcTestMethod(
VariableExpr actualResponseVarExpr =
VariableExpr.withVariable(
Variable.builder()
.setType(methodOutputType)
.setType(
method.isPaged() ? getPagedResponseType(service, method) : methodOutputType)
.setName(method.isPaged() ? "pagedListResponse" : "actualResponse")
.build());
Expr rpcJavaMethodInvocationExpr =
Expand Down Expand Up @@ -699,12 +710,21 @@ private static MethodDefinition createRpcTestMethod(
.build());

// Assert the responses are equivalent.
Message methodOutputMessage = messageTypes.get(method.outputType().reference().name());
Field firstRepeatedField = methodOutputMessage.findAndUnwrapFirstRepeatedField();
Preconditions.checkNotNull(
firstRepeatedField,
String.format(
"Expected paged RPC %s to have a repeated field in the response %s but found none",
method.name(), methodOutputMessage.name()));

Expr zeroExpr =
ValueExpr.withValue(PrimitiveValue.builder().setType(TypeNode.INT).setValue("0").build());
Expr expectedPagedResponseExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(expectedResponseVarExpr)
.setMethodName("getResponsesList")
.setMethodName(
String.format("get%sList", JavaStyle.toUpperCamelCase(firstRepeatedField.name())))
.build();
expectedPagedResponseExpr =
MethodInvocationExpr.builder()
Expand Down Expand Up @@ -1160,12 +1180,14 @@ private static MethodDefinition createStreamingRpcTestMethod(

private static MethodDefinition createRpcExceptionTestMethod(
Method method,
Service service,
List<MethodArgument> methodSignature,
int variantIndex,
String serviceName,
Map<String, VariableExpr> classMemberVarExprs,
Map<String, ResourceName> resourceNames,
Map<String, Message> messageTypes) {
String serviceName = service.name();

VariableExpr exceptionVarExpr =
VariableExpr.withVariable(
Variable.builder()
Expand Down Expand Up @@ -1833,6 +1855,16 @@ private static TypeNode getCallableType(Method protoMethod) {
ConcreteReference.builder().setClazz(callableClazz).setGenerics(generics).build());
}

private static TypeNode getPagedResponseType(Service service, Method method) {
return TypeNode.withReference(
VaporReference.builder()
.setName(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, method.name()))
.setPakkage(service.pakkage())
.setEnclosingClassName(getClientClassName(service.name()))
.setIsStaticImport(true)
.build());
}

private static String getCallableMethodName(Method protoMethod) {
Preconditions.checkState(
!protoMethod.stream().equals(Method.Stream.NONE),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.google.showcase.v1beta1;

import static com.google.showcase.v1beta1.EchoClient.PagedExpandPagedResponse;

import com.google.api.gax.core.NoCredentialsProvider;
import com.google.api.gax.grpc.GaxGrpcProperties;
import com.google.api.gax.grpc.testing.LocalChannelProvider;
Expand Down Expand Up @@ -550,7 +552,7 @@ public class EchoClientTest {
.setPageToken("page_token1630607433")
.build();

PagedExpandResponse pagedListResponse = client.pagedExpand(request);
PagedExpandPagedResponse pagedListResponse = client.pagedExpand(request);

List<EchoResponse> resources = Lists.newArrayList(pagedListResponse.iterateAll());

Expand Down
31 changes: 17 additions & 14 deletions test/integration/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,22 @@ load(

package(default_visibility = ["//visibility:public"])

####################################################
# Integration Test Rules
####################################################

integration_test(
name = "redis",
data = ["//test/integration/goldens/redis:goldens_files"],
target = ":redis_java_gapic",
)

integration_test(
name = "asset",
target = ":asset_java_gapic",
data = ["//test/integration/goldens/asset:goldens_files"],
)

####################################################
# API Library Rules
####################################################
Expand Down Expand Up @@ -53,17 +69,4 @@ java_gapic_library(
deps = [
"@com_google_googleapis//google/cloud/redis/v1:redis_java_proto",
],
)

integration_test(
name = "redis",
data = ["//test/integration/goldens/redis:goldens_files"],
target = ":redis_java_gapic",
)

integration_test(
name = "asset",
target = ":asset_java_gapic",
data = ["//test/integration/goldens/asset:goldens_files"],
)

)

0 comments on commit 3cf3ba3

Please sign in to comment.