Skip to content

Commit 348837e

Browse files
miraleungmiraleung
authored
[ggj][engx] feat!: remove all per-RPC batching codegen logic (#476)
* fix: swap assertEquals args in JavaWriterVisitorTest to match (expected, actusl) order * fix: swap assertEquals args in ImportWriterVisitorTest to match (expected, actusl) order * fix: add node validator to refactor/centralize null element checks * test: update logging integration goldens with gapic.yaml * feat!: remove gapic.yaml from Bazel and input interface * feat!: remove all per-RPC batching codegen logic * fix: update tests
1 parent 9652f7a commit 348837e

26 files changed

Lines changed: 54 additions & 2546 deletions

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

Lines changed: 0 additions & 557 deletions
This file was deleted.

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

Lines changed: 0 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,6 @@
1414

1515
package com.google.api.generator.gapic.composer;
1616

17-
import com.google.api.gax.batching.BatchingSettings;
18-
import com.google.api.gax.batching.FlowControlSettings;
19-
import com.google.api.gax.batching.FlowController;
2017
import com.google.api.gax.grpc.ProtoOperationTransformers;
2118
import com.google.api.gax.longrunning.OperationSnapshot;
2219
import com.google.api.gax.longrunning.OperationTimedPollAlgorithm;
@@ -37,7 +34,6 @@
3734
import com.google.api.generator.engine.ast.ValueExpr;
3835
import com.google.api.generator.engine.ast.Variable;
3936
import com.google.api.generator.engine.ast.VariableExpr;
40-
import com.google.api.generator.gapic.model.GapicBatchingSettings;
4137
import com.google.api.generator.gapic.model.GapicRetrySettings;
4238
import com.google.api.generator.gapic.model.GapicServiceConfig;
4339
import com.google.api.generator.gapic.model.Method;
@@ -368,108 +364,6 @@ public static Expr createLroSettingsBuilderExpr(
368364
return builderSettingsExpr;
369365
}
370366

371-
public static Expr createBatchingBuilderSettingsExpr(
372-
String settingsGetterMethodName,
373-
GapicBatchingSettings batchingSettings,
374-
VariableExpr builderVarExpr) {
375-
376-
Expr batchingSettingsBuilderExpr =
377-
MethodInvocationExpr.builder()
378-
.setStaticReferenceType(STATIC_TYPES.get("BatchingSettings"))
379-
.setMethodName("newBuilder")
380-
.build();
381-
382-
batchingSettingsBuilderExpr =
383-
MethodInvocationExpr.builder()
384-
.setExprReferenceExpr(batchingSettingsBuilderExpr)
385-
.setMethodName("setElementCountThreshold")
386-
.setArguments(toValExpr(batchingSettings.elementCountThreshold()))
387-
.build();
388-
389-
batchingSettingsBuilderExpr =
390-
MethodInvocationExpr.builder()
391-
.setExprReferenceExpr(batchingSettingsBuilderExpr)
392-
.setMethodName("setRequestByteThreshold")
393-
.setArguments(toValExpr(batchingSettings.requestByteThreshold()))
394-
.build();
395-
396-
batchingSettingsBuilderExpr =
397-
MethodInvocationExpr.builder()
398-
.setExprReferenceExpr(batchingSettingsBuilderExpr)
399-
.setMethodName("setDelayThreshold")
400-
.setArguments(
401-
createDurationOfMillisExpr(toValExpr(batchingSettings.delayThresholdMillis())))
402-
.build();
403-
404-
// FlowControlSettings.
405-
Expr flowControlSettingsExpr =
406-
MethodInvocationExpr.builder()
407-
.setStaticReferenceType(STATIC_TYPES.get("FlowControlSettings"))
408-
.setMethodName("newBuilder")
409-
.build();
410-
if (batchingSettings.flowControlElementLimit() != null) {
411-
flowControlSettingsExpr =
412-
MethodInvocationExpr.builder()
413-
.setExprReferenceExpr(flowControlSettingsExpr)
414-
.setMethodName("setMaxOutstandingElementCount")
415-
.setArguments(toValExpr(batchingSettings.flowControlElementLimit()))
416-
.build();
417-
}
418-
if (batchingSettings.flowControlByteLimit() != null) {
419-
flowControlSettingsExpr =
420-
MethodInvocationExpr.builder()
421-
.setExprReferenceExpr(flowControlSettingsExpr)
422-
.setMethodName("setMaxOutstandingRequestBytes")
423-
.setArguments(toValExpr(batchingSettings.flowControlByteLimit()))
424-
.build();
425-
}
426-
flowControlSettingsExpr =
427-
MethodInvocationExpr.builder()
428-
.setExprReferenceExpr(flowControlSettingsExpr)
429-
.setMethodName("setLimitExceededBehavior")
430-
.setArguments(
431-
EnumRefExpr.builder()
432-
.setType(STATIC_TYPES.get("LimitExceededBehavior"))
433-
.setName(
434-
JavaStyle.toUpperCamelCase(
435-
batchingSettings
436-
.flowControlLimitExceededBehavior()
437-
.name()
438-
.toLowerCase()))
439-
.build())
440-
.build();
441-
flowControlSettingsExpr =
442-
MethodInvocationExpr.builder()
443-
.setExprReferenceExpr(flowControlSettingsExpr)
444-
.setMethodName("build")
445-
.build();
446-
447-
batchingSettingsBuilderExpr =
448-
MethodInvocationExpr.builder()
449-
.setExprReferenceExpr(batchingSettingsBuilderExpr)
450-
.setMethodName("setFlowControlSettings")
451-
.setArguments(flowControlSettingsExpr)
452-
.build();
453-
454-
batchingSettingsBuilderExpr =
455-
MethodInvocationExpr.builder()
456-
.setExprReferenceExpr(batchingSettingsBuilderExpr)
457-
.setMethodName("build")
458-
.build();
459-
460-
// Put everything together.
461-
Expr builderExpr =
462-
MethodInvocationExpr.builder()
463-
.setExprReferenceExpr(builderVarExpr)
464-
.setMethodName(settingsGetterMethodName)
465-
.build();
466-
return MethodInvocationExpr.builder()
467-
.setExprReferenceExpr(builderExpr)
468-
.setMethodName("setBatchingSettings")
469-
.setArguments(batchingSettingsBuilderExpr)
470-
.build();
471-
}
472-
473367
private static Expr createRetryCodeDefinitionExpr(
474368
String codeName, List<Code> retryCodes, VariableExpr definitionsVarExpr) {
475369
// Construct something like `definitions.put("code_name",
@@ -705,10 +599,7 @@ private static MethodInvocationExpr createDurationOfMillisExpr(ValueExpr valExpr
705599
private static Map<String, TypeNode> createStaticTypes() {
706600
List<Class> concreteClazzes =
707601
Arrays.asList(
708-
BatchingSettings.class,
709602
org.threeten.bp.Duration.class,
710-
FlowControlSettings.class,
711-
FlowController.LimitExceededBehavior.class,
712603
ImmutableMap.class,
713604
ImmutableSet.class,
714605
Lists.class,

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

Lines changed: 14 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,6 @@
1818
import com.google.api.core.ApiFunction;
1919
import com.google.api.core.ApiFuture;
2020
import com.google.api.core.BetaApi;
21-
import com.google.api.gax.batching.BatchingSettings;
22-
import com.google.api.gax.batching.FlowControlSettings;
23-
import com.google.api.gax.batching.FlowController.LimitExceededBehavior;
24-
import com.google.api.gax.batching.PartitionKey;
25-
import com.google.api.gax.batching.RequestBuilder;
2621
import com.google.api.gax.core.GaxProperties;
2722
import com.google.api.gax.core.GoogleCredentialsProvider;
2823
import com.google.api.gax.core.InstantiatingExecutorProvider;
@@ -35,9 +30,6 @@
3530
import com.google.api.gax.retrying.RetrySettings;
3631
import com.google.api.gax.rpc.ApiCallContext;
3732
import com.google.api.gax.rpc.ApiClientHeaderProvider;
38-
import com.google.api.gax.rpc.BatchedRequestIssuer;
39-
import com.google.api.gax.rpc.BatchingCallSettings;
40-
import com.google.api.gax.rpc.BatchingDescriptor;
4133
import com.google.api.gax.rpc.ClientContext;
4234
import com.google.api.gax.rpc.OperationCallSettings;
4335
import com.google.api.gax.rpc.PageContext;
@@ -82,7 +74,6 @@
8274
import com.google.api.generator.engine.ast.Variable;
8375
import com.google.api.generator.engine.ast.VariableExpr;
8476
import com.google.api.generator.gapic.model.Field;
85-
import com.google.api.generator.gapic.model.GapicBatchingSettings;
8677
import com.google.api.generator.gapic.model.GapicClass;
8778
import com.google.api.generator.gapic.model.GapicServiceConfig;
8879
import com.google.api.generator.gapic.model.Message;
@@ -221,12 +212,8 @@ private static Map<String, VariableExpr> createMethodSettingsClassMemberVarExprs
221212
Map<String, VariableExpr> varExprs = new LinkedHashMap<>();
222213

223214
// Creates class variables <method>Settings, e.g. echoSettings.
224-
// TODO(miraleung): Handle batching here.
225215
for (Method method : service.methods()) {
226-
boolean hasBatchingSettings =
227-
!Objects.isNull(serviceConfig) && serviceConfig.hasBatchingSetting(service, method);
228-
TypeNode settingsType =
229-
getCallSettingsType(method, types, hasBatchingSettings, isNestedClass);
216+
TypeNode settingsType = getCallSettingsType(method, types, isNestedClass);
230217
String varName = JavaStyle.toLowerCamelCase(String.format("%sSettings", method.name()));
231218
varExprs.put(
232219
varName,
@@ -313,20 +300,6 @@ private static List<Statement> createClassStatements(
313300
statements.add(EMPTY_LINE_STATEMENT);
314301
}
315302

316-
for (Method method : service.methods()) {
317-
Optional<GapicBatchingSettings> batchingSettingOpt =
318-
Objects.isNull(serviceConfig)
319-
? Optional.empty()
320-
: serviceConfig.getBatchingSetting(service, method);
321-
if (batchingSettingOpt.isPresent()) {
322-
statements.add(
323-
exprToStatementFn.apply(
324-
BatchingDescriptorComposer.createBatchingDescriptorFieldDeclExpr(
325-
method, batchingSettingOpt.get(), messageTypes)));
326-
}
327-
statements.add(EMPTY_LINE_STATEMENT);
328-
}
329-
330303
return statements;
331304
}
332305

@@ -1261,22 +1234,6 @@ private static MethodDefinition createNestedClassInitDefaultsMethod(
12611234
if (streamKind.equals(Method.Stream.CLIENT) || streamKind.equals(Method.Stream.BIDI)) {
12621235
continue;
12631236
}
1264-
if (!Objects.isNull(serviceConfig) && serviceConfig.hasBatchingSetting(service, method)) {
1265-
Optional<GapicBatchingSettings> batchingSettingOpt =
1266-
serviceConfig.getBatchingSetting(service, method);
1267-
Preconditions.checkState(
1268-
batchingSettingOpt.isPresent(),
1269-
String.format(
1270-
"No batching setting found for service %s, method %s",
1271-
service.name(), method.name()));
1272-
String settingsGetterMethodName =
1273-
String.format("%sSettings", JavaStyle.toLowerCamelCase(method.name()));
1274-
bodyStatements.add(
1275-
ExprStatement.withExpr(
1276-
RetrySettingsComposer.createBatchingBuilderSettingsExpr(
1277-
settingsGetterMethodName, batchingSettingOpt.get(), builderVarExpr)));
1278-
bodyStatements.add(EMPTY_LINE_STATEMENT);
1279-
}
12801237

12811238
bodyStatements.add(
12821239
ExprStatement.withExpr(
@@ -1352,8 +1309,6 @@ private static List<MethodDefinition> createNestedClassConstructorMethods(
13521309
.build());
13531310
Reference pagedSettingsBuilderRef =
13541311
ConcreteReference.withClazz(PagedCallSettings.Builder.class);
1355-
Reference batchingSettingsBuilderRef =
1356-
ConcreteReference.withClazz(BatchingCallSettings.Builder.class);
13571312
Reference unaryCallSettingsBuilderRef =
13581313
ConcreteReference.withClazz(UnaryCallSettings.Builder.class);
13591314
Function<TypeNode, Boolean> isUnaryCallSettingsBuilderFn =
@@ -1363,9 +1318,6 @@ private static List<MethodDefinition> createNestedClassConstructorMethods(
13631318
.equals(unaryCallSettingsBuilderRef);
13641319
Function<TypeNode, Boolean> isPagedCallSettingsBuilderFn =
13651320
t -> t.reference().copyAndSetGenerics(ImmutableList.of()).equals(pagedSettingsBuilderRef);
1366-
Function<TypeNode, Boolean> isBatchingCallSettingsBuilderFn =
1367-
t ->
1368-
t.reference().copyAndSetGenerics(ImmutableList.of()).equals(batchingSettingsBuilderRef);
13691321
Function<TypeNode, TypeNode> builderToCallSettingsFn =
13701322
t ->
13711323
TypeNode.withReference(
@@ -1396,61 +1348,20 @@ private static List<MethodDefinition> createNestedClassConstructorMethods(
13961348
String methodName = getMethodNameFromSettingsVarName(e.getKey());
13971349

13981350
if (!isPagedCallSettingsBuilderFn.apply(varType)) {
1399-
if (!isBatchingCallSettingsBuilderFn.apply(varType)) {
1400-
boolean isUnaryCallSettings = isUnaryCallSettingsBuilderFn.apply(varType);
1401-
Expr builderExpr =
1402-
AssignmentExpr.builder()
1403-
.setVariableExpr(varExpr)
1404-
.setValueExpr(
1405-
MethodInvocationExpr.builder()
1406-
.setStaticReferenceType(
1407-
builderToCallSettingsFn.apply(varExpr.type()))
1408-
.setMethodName(
1409-
isUnaryCallSettings
1410-
? "newUnaryCallSettingsBuilder"
1411-
: "newBuilder")
1412-
.setReturnType(varExpr.type())
1413-
.build())
1414-
.build();
1415-
return ExprStatement.withExpr(builderExpr);
1416-
}
1417-
Expr newBatchingSettingsExpr =
1418-
MethodInvocationExpr.builder()
1419-
.setStaticReferenceType(STATIC_TYPES.get("BatchingSettings"))
1420-
.setMethodName("newBuilder")
1421-
.build();
1422-
newBatchingSettingsExpr =
1423-
MethodInvocationExpr.builder()
1424-
.setExprReferenceExpr(newBatchingSettingsExpr)
1425-
.setMethodName("build")
1426-
.build();
1427-
1428-
String batchingDescVarName =
1429-
String.format(
1430-
BATCHING_DESC_PATTERN, JavaStyle.toUpperSnakeCase(methodName));
1431-
Expr batchingSettingsBuilderExpr =
1432-
MethodInvocationExpr.builder()
1433-
.setStaticReferenceType(builderToCallSettingsFn.apply(varType))
1434-
.setMethodName("newBuilder")
1435-
.setArguments(
1436-
VariableExpr.withVariable(
1437-
Variable.builder()
1438-
.setType(STATIC_TYPES.get("BatchingDescriptor"))
1439-
.setName(batchingDescVarName)
1440-
.build()))
1441-
.build();
1442-
batchingSettingsBuilderExpr =
1443-
MethodInvocationExpr.builder()
1444-
.setExprReferenceExpr(batchingSettingsBuilderExpr)
1445-
.setMethodName("setBatchingSettings")
1446-
.setArguments(newBatchingSettingsExpr)
1447-
.setReturnType(varType)
1448-
.build();
1449-
1351+
boolean isUnaryCallSettings = isUnaryCallSettingsBuilderFn.apply(varType);
14501352
Expr builderExpr =
14511353
AssignmentExpr.builder()
14521354
.setVariableExpr(varExpr)
1453-
.setValueExpr(batchingSettingsBuilderExpr)
1355+
.setValueExpr(
1356+
MethodInvocationExpr.builder()
1357+
.setStaticReferenceType(
1358+
builderToCallSettingsFn.apply(varExpr.type()))
1359+
.setMethodName(
1360+
isUnaryCallSettings
1361+
? "newUnaryCallSettingsBuilder"
1362+
: "newBuilder")
1363+
.setReturnType(varExpr.type())
1364+
.build())
14541365
.build();
14551366
return ExprStatement.withExpr(builderExpr);
14561367
}
@@ -1496,8 +1407,7 @@ private static List<MethodDefinition> createNestedClassConstructorMethods(
14961407
.filter(
14971408
v ->
14981409
isUnaryCallSettingsBuilderFn.apply(v.type())
1499-
|| isPagedCallSettingsBuilderFn.apply(v.type())
1500-
|| isBatchingCallSettingsBuilderFn.apply(v.type()))
1410+
|| isPagedCallSettingsBuilderFn.apply(v.type()))
15011411
.collect(Collectors.toList()))
15021412
.setReturnType(NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR.type())
15031413
.build())
@@ -1779,15 +1689,10 @@ private static Map<String, TypeNode> createStaticTypes() {
17791689
ApiClientHeaderProvider.class,
17801690
ApiFunction.class,
17811691
ApiFuture.class,
1782-
BatchedRequestIssuer.class,
1783-
BatchingCallSettings.class,
1784-
BatchingDescriptor.class,
1785-
BatchingSettings.class,
17861692
BetaApi.class,
17871693
ClientContext.class,
17881694
Duration.class,
17891695
Empty.class,
1790-
FlowControlSettings.class,
17911696
GaxGrpcProperties.class,
17921697
GaxProperties.class,
17931698
Generated.class,
@@ -1799,7 +1704,6 @@ private static Map<String, TypeNode> createStaticTypes() {
17991704
ImmutableSet.class,
18001705
InstantiatingExecutorProvider.class,
18011706
InstantiatingGrpcChannelProvider.class,
1802-
LimitExceededBehavior.class,
18031707
List.class,
18041708
Lists.class,
18051709
MonitoredResourceDescriptor.class,
@@ -1811,9 +1715,7 @@ private static Map<String, TypeNode> createStaticTypes() {
18111715
PagedCallSettings.class,
18121716
PagedListDescriptor.class,
18131717
PagedListResponseFactory.class,
1814-
PartitionKey.class,
18151718
ProtoOperationTransformers.class,
1816-
RequestBuilder.class,
18171719
RetrySettings.class,
18181720
ServerStreamingCallSettings.class,
18191721
StatusCode.class,
@@ -1969,10 +1871,7 @@ private static String getGrpcServiceStubTypeName(String serviceName) {
19691871
}
19701872

19711873
private static TypeNode getCallSettingsType(
1972-
Method method,
1973-
Map<String, TypeNode> types,
1974-
boolean isBatchingSettings,
1975-
final boolean isSettingsBuilder) {
1874+
Method method, Map<String, TypeNode> types, final boolean isSettingsBuilder) {
19761875
Function<Class, TypeNode> typeMakerFn =
19771876
clz -> TypeNode.withReference(ConcreteReference.withClazz(clz));
19781877
// Default: No streaming.
@@ -1982,11 +1881,6 @@ private static TypeNode getCallSettingsType(
19821881
isSettingsBuilder ? PagedCallSettings.Builder.class : PagedCallSettings.class)
19831882
: typeMakerFn.apply(
19841883
isSettingsBuilder ? UnaryCallSettings.Builder.class : UnaryCallSettings.class);
1985-
if (isBatchingSettings) {
1986-
callSettingsType =
1987-
typeMakerFn.apply(
1988-
isSettingsBuilder ? BatchingCallSettings.Builder.class : BatchingCallSettings.class);
1989-
}
19901884

19911885
// Streaming takes precendence over paging, as per the monolith's existing behavior.
19921886
switch (method.stream()) {

0 commit comments

Comments
 (0)