Skip to content

Commit b0beddc

Browse files
miraleungmiraleung
committed
fix(mixins): handle unit tests for mixin pagination methods
1 parent 6f8ca38 commit b0beddc

12 files changed

Lines changed: 803 additions & 19 deletions

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

Lines changed: 65 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,7 @@ public GapicClass generate(GapicContext context, Service service) {
140140
Map<String, Message> messageTypes = context.messages();
141141
String pakkage = service.pakkage();
142142
TypeStore typeStore = new TypeStore();
143-
addDynamicTypes(service, typeStore);
144-
for (Service mixinService : context.mixinServices()) {
145-
addDynamicTypes(mixinService, typeStore);
146-
}
147-
143+
addDynamicTypes(context, service, typeStore);
148144
String className = ClassNames.getServiceClientTestClassName(service);
149145
GapicClass.Kind kind = Kind.MAIN;
150146

@@ -466,6 +462,7 @@ private static List<MethodDefinition> createTestMethods(
466462
javaMethods.add(
467463
createRpcTestMethod(
468464
method,
465+
service,
469466
matchingService,
470467
Collections.emptyList(),
471468
0,
@@ -487,6 +484,7 @@ private static List<MethodDefinition> createTestMethods(
487484
javaMethods.add(
488485
createRpcTestMethod(
489486
method,
487+
service,
490488
matchingService,
491489
method.methodSignatures().get(i),
492490
i,
@@ -509,9 +507,27 @@ private static List<MethodDefinition> createTestMethods(
509507
return javaMethods;
510508
}
511509

510+
/**
511+
* Creates a test method for a given RPC, e.g. createAssetTest.
512+
*
513+
* @param method the RPC for which this test method is created.
514+
* @param apiService the host service under test.
515+
* @param rpcService the service that {@code method} belongs to. This is not equal to {@code
516+
* apiService} only when {@code method} is a mixin, in which case {@code rpcService} is the
517+
* mixed-in service. If {@code apiService} and {@code rpcService} are different, they will be
518+
* used only for pagination. Otherwise, {@code rpcService} subsumes {@code apiService}.
519+
* @param methodSignature the method signature of the RPC under test.
520+
* @param variantIndex the nth variant of the RPC under test. This applies when we have
521+
* polymorphism due to the presence of several method signature annotations in the proto.
522+
* @param isRequestArg whether the RPC variant under test take only the request proto message.
523+
* @param classMemberVarExprs the class members in the generated test class.
524+
* @param resourceNames the resource names available for use.
525+
* @param messageTypes the proto message types available for use.
526+
*/
512527
private static MethodDefinition createRpcTestMethod(
513528
Method method,
514-
Service service,
529+
Service apiService,
530+
Service rpcService,
515531
List<MethodArgument> methodSignature,
516532
int variantIndex,
517533
boolean isRequestArg,
@@ -520,14 +536,15 @@ private static MethodDefinition createRpcTestMethod(
520536
Map<String, Message> messageTypes) {
521537
if (!method.stream().equals(Method.Stream.NONE)) {
522538
return createStreamingRpcTestMethod(
523-
service, method, classMemberVarExprs, resourceNames, messageTypes);
539+
rpcService, method, classMemberVarExprs, resourceNames, messageTypes);
524540
}
525541
// Construct the expected response.
526542
TypeNode methodOutputType = method.hasLro() ? method.lro().responseType() : method.outputType();
527543
List<Expr> methodExprs = new ArrayList<>();
528544

529545
TypeNode repeatedResponseType = null;
530546
VariableExpr responsesElementVarExpr = null;
547+
String mockServiceVarName = getMockServiceVarName(rpcService);
531548
if (method.isPaged()) {
532549
Message methodOutputMessage = messageTypes.get(method.outputType().reference().simpleName());
533550
Field repeatedPagedResultsField = methodOutputMessage.findAndUnwrapFirstRepeatedField();
@@ -596,6 +613,7 @@ private static MethodDefinition createRpcTestMethod(
596613
.setVariableExpr(expectedResponseVarExpr.toBuilder().setIsDecl(true).build())
597614
.setValueExpr(expectedResponseValExpr)
598615
.build());
616+
599617
if (method.hasLro()) {
600618
VariableExpr resultOperationVarExpr =
601619
VariableExpr.withVariable(
@@ -613,14 +631,14 @@ private static MethodDefinition createRpcTestMethod(
613631
.build());
614632
methodExprs.add(
615633
MethodInvocationExpr.builder()
616-
.setExprReferenceExpr(classMemberVarExprs.get(getMockServiceVarName(service)))
634+
.setExprReferenceExpr(classMemberVarExprs.get(mockServiceVarName))
617635
.setMethodName("addResponse")
618636
.setArguments(resultOperationVarExpr)
619637
.build());
620638
} else {
621639
methodExprs.add(
622640
MethodInvocationExpr.builder()
623-
.setExprReferenceExpr(classMemberVarExprs.get(getMockServiceVarName(service)))
641+
.setExprReferenceExpr(classMemberVarExprs.get(mockServiceVarName))
624642
.setMethodName("addResponse")
625643
.setArguments(expectedResponseVarExpr)
626644
.build());
@@ -675,7 +693,12 @@ private static MethodDefinition createRpcTestMethod(
675693
VariableExpr.withVariable(
676694
Variable.builder()
677695
.setType(
678-
method.isPaged() ? getPagedResponseType(method, service) : methodOutputType)
696+
!method.isPaged()
697+
? methodOutputType
698+
// If this method is a paginated mixin, use the host service, since
699+
// ServiceClient defines the paged response class and the mixed-in service
700+
// does not have a client.
701+
: getPagedResponseType(method, method.isMixin() ? apiService : rpcService))
679702
.setName(method.isPaged() ? "pagedListResponse" : "actualResponse")
680703
.build());
681704
Expr rpcJavaMethodInvocationExpr =
@@ -828,7 +851,7 @@ private static MethodDefinition createRpcTestMethod(
828851
.setVariableExpr(actualRequestsVarExpr.toBuilder().setIsDecl(true).build())
829852
.setValueExpr(
830853
MethodInvocationExpr.builder()
831-
.setExprReferenceExpr(classMemberVarExprs.get(getMockServiceVarName(service)))
854+
.setExprReferenceExpr(classMemberVarExprs.get(mockServiceVarName))
832855
.setMethodName("getRequests")
833856
.setReturnType(actualRequestsVarExpr.type())
834857
.build())
@@ -1021,6 +1044,8 @@ private static MethodDefinition createStreamingRpcTestMethod(
10211044
.setVariableExpr(expectedResponseVarExpr.toBuilder().setIsDecl(true).build())
10221045
.setValueExpr(expectedResponseValExpr)
10231046
.build());
1047+
1048+
String mockServiceVarName = getMockServiceVarName(service);
10241049
if (method.hasLro()) {
10251050
VariableExpr resultOperationVarExpr =
10261051
VariableExpr.withVariable(
@@ -1038,14 +1063,14 @@ private static MethodDefinition createStreamingRpcTestMethod(
10381063
.build());
10391064
methodExprs.add(
10401065
MethodInvocationExpr.builder()
1041-
.setExprReferenceExpr(classMemberVarExprs.get(getMockServiceVarName(service)))
1066+
.setExprReferenceExpr(classMemberVarExprs.get(mockServiceVarName))
10421067
.setMethodName("addResponse")
10431068
.setArguments(resultOperationVarExpr)
10441069
.build());
10451070
} else {
10461071
methodExprs.add(
10471072
MethodInvocationExpr.builder()
1048-
.setExprReferenceExpr(classMemberVarExprs.get(getMockServiceVarName(service)))
1073+
.setExprReferenceExpr(classMemberVarExprs.get(mockServiceVarName))
10491074
.setMethodName("addResponse")
10501075
.setArguments(expectedResponseVarExpr)
10511076
.build());
@@ -1251,7 +1276,19 @@ private static MethodDefinition createStreamingRpcTestMethod(
12511276
.build();
12521277
}
12531278

1254-
// TODO(imraleung): Reorder params.
1279+
/**
1280+
* Creates a test method to exercise exceptions for a given RPC, e.g. createAssetTest.
1281+
*
1282+
* @param method the RPC for which this test method is created.
1283+
* @param service the service that {@code method} belongs to.
1284+
* @param methodSignature the method signature of the RPC under test.
1285+
* @param variantIndex the nth variant of the RPC under test. This applies when we have
1286+
* polymorphism due to the presence of several method signature annotations in the proto.
1287+
* @param classMemberVarExprs the class members in the generated test class.
1288+
* @param resourceNames the resource names available for use.
1289+
* @param messageTypes the proto message types available for use.
1290+
*/
1291+
// TODO(miraleung): Reorder params.
12551292
private static MethodDefinition createRpcExceptionTestMethod(
12561293
Method method,
12571294
Service service,
@@ -1759,7 +1796,7 @@ private static Map<String, TypeNode> createDefaultMethodNamesToTypes() {
17591796
return javaMethodNameToReturnType;
17601797
}
17611798

1762-
private static void addDynamicTypes(Service service, TypeStore typeStore) {
1799+
private static void addDynamicTypes(GapicContext context, Service service, TypeStore typeStore) {
17631800
typeStore.putAll(
17641801
service.pakkage(),
17651802
Arrays.asList(
@@ -1775,6 +1812,19 @@ private static void addDynamicTypes(Service service, TypeStore typeStore) {
17751812
.collect(Collectors.toList()),
17761813
true,
17771814
ClassNames.getServiceClientClassName(service));
1815+
for (Service mixinService : context.mixinServices()) {
1816+
typeStore.put(mixinService.pakkage(), ClassNames.getMockServiceClassName(mixinService));
1817+
for (Method mixinMethod : mixinService.methods()) {
1818+
if (!mixinMethod.isPaged()) {
1819+
continue;
1820+
}
1821+
typeStore.put(
1822+
service.pakkage(),
1823+
String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, mixinMethod.name()),
1824+
true,
1825+
ClassNames.getServiceClientClassName(service));
1826+
}
1827+
}
17781828
}
17791829

17801830
private static TypeNode getOperationCallSettingsType(Method protoMethod) {

‎test/integration/BUILD.bazel‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ load(
1212
"golden_update",
1313
"integration_test",
1414
)
15-
1615
load("@rules_proto//proto:defs.bzl", "proto_library")
1716

1817
package(default_visibility = ["//visibility:public"])
@@ -204,6 +203,7 @@ proto_library(
204203
"@com_google_googleapis//google/api:client_proto",
205204
"@com_google_googleapis//google/api:field_behavior_proto",
206205
"@com_google_googleapis//google/api:resource_proto",
206+
"@com_google_googleapis//google/cloud/location:location_proto",
207207
"@com_google_googleapis//google/iam/v1:iam_policy_proto",
208208
"@com_google_googleapis//google/iam/v1:policy_proto",
209209
"@com_google_protobuf//:duration_proto",
@@ -219,6 +219,7 @@ proto_library_with_info(
219219
deps = [
220220
":kms_proto",
221221
"@com_google_googleapis//google/cloud:common_resources_proto",
222+
"@com_google_googleapis//google/cloud/location:location_proto",
222223
"@com_google_googleapis//google/iam/v1:iam_policy_proto",
223224
"@com_google_googleapis//google/iam/v1:policy_proto",
224225
],
@@ -243,10 +244,12 @@ java_gapic_library(
243244
service_yaml = "apis/kms/v1/cloudkms_test_mixins_v1.yaml",
244245
test_deps = [
245246
":kms_java_grpc",
247+
"@com_google_googleapis//google/cloud/location:location_java_grpc",
246248
"@com_google_googleapis//google/iam/v1:iam_java_grpc",
247249
],
248250
deps = [
249251
":kms_java_proto",
252+
"@com_google_googleapis//google/cloud/location:location_java_proto",
250253
"@com_google_googleapis//google/iam/v1:iam_java_proto",
251254
],
252255
)
@@ -267,5 +270,7 @@ java_gapic_assembly_gradle_pkg(
267270
"@com_google_googleapis//google/cloud/kms/v1:kms_java_grpc",
268271
"@com_google_googleapis//google/cloud/kms/v1:kms_java_proto",
269272
"@com_google_googleapis//google/cloud/kms/v1:kms_proto",
273+
"@com_google_googleapis//google/cloud/location:location_java_grpc",
274+
"@com_google_googleapis//google/cloud/location:location_java_proto",
270275
],
271276
)

‎test/integration/apis/kms/v1/cloudkms_test_mixins_v1.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ title: Cloud Key Management Service (KMS) API
55

66
apis:
77
- name: google.cloud.kms.v1.KeyManagementService
8+
- name: google.cloud.location.Locations
89
- name: google.iam.v1.IAMPolicy
910

1011
types:

‎test/integration/goldens/kms/GrpcKeyManagementServiceStub.java‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static com.google.cloud.kms.v1.KeyManagementServiceClient.ListCryptoKeysPagedResponse;
2121
import static com.google.cloud.kms.v1.KeyManagementServiceClient.ListImportJobsPagedResponse;
2222
import static com.google.cloud.kms.v1.KeyManagementServiceClient.ListKeyRingsPagedResponse;
23+
import static com.google.cloud.kms.v1.KeyManagementServiceClient.ListLocationsPagedResponse;
2324

2425
import com.google.api.gax.core.BackgroundResource;
2526
import com.google.api.gax.core.BackgroundResourceAggregation;
@@ -64,6 +65,10 @@
6465
import com.google.cloud.kms.v1.UpdateCryptoKeyPrimaryVersionRequest;
6566
import com.google.cloud.kms.v1.UpdateCryptoKeyRequest;
6667
import com.google.cloud.kms.v1.UpdateCryptoKeyVersionRequest;
68+
import com.google.cloud.location.GetLocationRequest;
69+
import com.google.cloud.location.ListLocationsRequest;
70+
import com.google.cloud.location.ListLocationsResponse;
71+
import com.google.cloud.location.Location;
6772
import com.google.common.collect.ImmutableMap;
6873
import com.google.iam.v1.GetIamPolicyRequest;
6974
import com.google.iam.v1.Policy;
@@ -340,6 +345,25 @@ public class GrpcKeyManagementServiceStub extends KeyManagementServiceStub {
340345
ProtoUtils.marshaller(TestIamPermissionsResponse.getDefaultInstance()))
341346
.build();
342347

348+
private static final MethodDescriptor<ListLocationsRequest, ListLocationsResponse>
349+
listLocationsMethodDescriptor =
350+
MethodDescriptor.<ListLocationsRequest, ListLocationsResponse>newBuilder()
351+
.setType(MethodDescriptor.MethodType.UNARY)
352+
.setFullMethodName("google.cloud.location.Locations/ListLocations")
353+
.setRequestMarshaller(
354+
ProtoUtils.marshaller(ListLocationsRequest.getDefaultInstance()))
355+
.setResponseMarshaller(
356+
ProtoUtils.marshaller(ListLocationsResponse.getDefaultInstance()))
357+
.build();
358+
359+
private static final MethodDescriptor<GetLocationRequest, Location> getLocationMethodDescriptor =
360+
MethodDescriptor.<GetLocationRequest, Location>newBuilder()
361+
.setType(MethodDescriptor.MethodType.UNARY)
362+
.setFullMethodName("google.cloud.location.Locations/GetLocation")
363+
.setRequestMarshaller(ProtoUtils.marshaller(GetLocationRequest.getDefaultInstance()))
364+
.setResponseMarshaller(ProtoUtils.marshaller(Location.getDefaultInstance()))
365+
.build();
366+
343367
private final UnaryCallable<ListKeyRingsRequest, ListKeyRingsResponse> listKeyRingsCallable;
344368
private final UnaryCallable<ListKeyRingsRequest, ListKeyRingsPagedResponse>
345369
listKeyRingsPagedCallable;
@@ -384,6 +408,10 @@ public class GrpcKeyManagementServiceStub extends KeyManagementServiceStub {
384408
private final UnaryCallable<SetIamPolicyRequest, Policy> setIamPolicyCallable;
385409
private final UnaryCallable<TestIamPermissionsRequest, TestIamPermissionsResponse>
386410
testIamPermissionsCallable;
411+
private final UnaryCallable<ListLocationsRequest, ListLocationsResponse> listLocationsCallable;
412+
private final UnaryCallable<ListLocationsRequest, ListLocationsPagedResponse>
413+
listLocationsPagedCallable;
414+
private final UnaryCallable<GetLocationRequest, Location> getLocationCallable;
387415

388416
private final BackgroundResource backgroundResources;
389417
private final GrpcOperationsStub operationsStub;
@@ -784,6 +812,32 @@ public Map<String, String> extract(TestIamPermissionsRequest request) {
784812
}
785813
})
786814
.build();
815+
GrpcCallSettings<ListLocationsRequest, ListLocationsResponse> listLocationsTransportSettings =
816+
GrpcCallSettings.<ListLocationsRequest, ListLocationsResponse>newBuilder()
817+
.setMethodDescriptor(listLocationsMethodDescriptor)
818+
.setParamsExtractor(
819+
new RequestParamsExtractor<ListLocationsRequest>() {
820+
@Override
821+
public Map<String, String> extract(ListLocationsRequest request) {
822+
ImmutableMap.Builder<String, String> params = ImmutableMap.builder();
823+
params.put("name", String.valueOf(request.getName()));
824+
return params.build();
825+
}
826+
})
827+
.build();
828+
GrpcCallSettings<GetLocationRequest, Location> getLocationTransportSettings =
829+
GrpcCallSettings.<GetLocationRequest, Location>newBuilder()
830+
.setMethodDescriptor(getLocationMethodDescriptor)
831+
.setParamsExtractor(
832+
new RequestParamsExtractor<GetLocationRequest>() {
833+
@Override
834+
public Map<String, String> extract(GetLocationRequest request) {
835+
ImmutableMap.Builder<String, String> params = ImmutableMap.builder();
836+
params.put("name", String.valueOf(request.getName()));
837+
return params.build();
838+
}
839+
})
840+
.build();
787841

788842
this.listKeyRingsCallable =
789843
callableFactory.createUnaryCallable(
@@ -897,6 +951,15 @@ public Map<String, String> extract(TestIamPermissionsRequest request) {
897951
testIamPermissionsTransportSettings,
898952
settings.testIamPermissionsSettings(),
899953
clientContext);
954+
this.listLocationsCallable =
955+
callableFactory.createUnaryCallable(
956+
listLocationsTransportSettings, settings.listLocationsSettings(), clientContext);
957+
this.listLocationsPagedCallable =
958+
callableFactory.createPagedCallable(
959+
listLocationsTransportSettings, settings.listLocationsSettings(), clientContext);
960+
this.getLocationCallable =
961+
callableFactory.createUnaryCallable(
962+
getLocationTransportSettings, settings.getLocationSettings(), clientContext);
900963

901964
this.backgroundResources =
902965
new BackgroundResourceAggregation(clientContext.getBackgroundResources());
@@ -1068,6 +1131,22 @@ public UnaryCallable<SetIamPolicyRequest, Policy> setIamPolicyCallable() {
10681131
return testIamPermissionsCallable;
10691132
}
10701133

1134+
@Override
1135+
public UnaryCallable<ListLocationsRequest, ListLocationsResponse> listLocationsCallable() {
1136+
return listLocationsCallable;
1137+
}
1138+
1139+
@Override
1140+
public UnaryCallable<ListLocationsRequest, ListLocationsPagedResponse>
1141+
listLocationsPagedCallable() {
1142+
return listLocationsPagedCallable;
1143+
}
1144+
1145+
@Override
1146+
public UnaryCallable<GetLocationRequest, Location> getLocationCallable() {
1147+
return getLocationCallable;
1148+
}
1149+
10711150
@Override
10721151
public final void close() {
10731152
shutdown();

0 commit comments

Comments
 (0)