Skip to content

[ggj][engx] feat!: remove all per-RPC batching codegen logic #476

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

Merged
miraleung merged 9 commits into from
Nov 12, 2020
Merged

[ggj][engx] feat!: remove all per-RPC batching codegen logic #476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0725750
fix: swap assertEquals args in JavaWriterVisitorTest to match (expect…
miraleung Nov 7, 2020
67f3a15
fix: swap assertEquals args in ImportWriterVisitorTest to match (expe…
miraleung Nov 7, 2020
1e1a86b
fix: add node validator to refactor/centralize null element checks
miraleung Nov 7, 2020
0a6662c
test: update logging integration goldens with gapic.yaml
miraleung Nov 10, 2020
e9cdf58
feat!: remove gapic.yaml from Bazel and input interface
miraleung Nov 10, 2020
827049d
feat!: remove all per-RPC batching codegen logic
miraleung Nov 10, 2020
ea153c5
fix: update tests
miraleung Nov 10, 2020
7f15ea5
Merge branch 'gp/g1' of github.com:googleapis/gapic-generator-java in…
miraleung Nov 10, 2020
d842b0d
Merge branch 'master' into gp/g2
miraleung Nov 12, 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
557 changes: 0 additions & 557 deletions src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java
View file
Open in desktop

This file was deleted.

109 changes: 0 additions & 109 deletions src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@

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

import com.google.api.gax.batching.BatchingSettings;
import com.google.api.gax.batching.FlowControlSettings;
import com.google.api.gax.batching.FlowController;
import com.google.api.gax.grpc.ProtoOperationTransformers;
import com.google.api.gax.longrunning.OperationSnapshot;
import com.google.api.gax.longrunning.OperationTimedPollAlgorithm;
Expand All @@ -37,7 +34,6 @@
import com.google.api.generator.engine.ast.ValueExpr;
import com.google.api.generator.engine.ast.Variable;
import com.google.api.generator.engine.ast.VariableExpr;
import com.google.api.generator.gapic.model.GapicBatchingSettings;
import com.google.api.generator.gapic.model.GapicRetrySettings;
import com.google.api.generator.gapic.model.GapicServiceConfig;
import com.google.api.generator.gapic.model.Method;
Expand Down Expand Up @@ -368,108 +364,6 @@ public static Expr createLroSettingsBuilderExpr(
return builderSettingsExpr;
}

public static Expr createBatchingBuilderSettingsExpr(
String settingsGetterMethodName,
GapicBatchingSettings batchingSettings,
VariableExpr builderVarExpr) {

Expr batchingSettingsBuilderExpr =
MethodInvocationExpr.builder()
.setStaticReferenceType(STATIC_TYPES.get("BatchingSettings"))
.setMethodName("newBuilder")
.build();

batchingSettingsBuilderExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(batchingSettingsBuilderExpr)
.setMethodName("setElementCountThreshold")
.setArguments(toValExpr(batchingSettings.elementCountThreshold()))
.build();

batchingSettingsBuilderExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(batchingSettingsBuilderExpr)
.setMethodName("setRequestByteThreshold")
.setArguments(toValExpr(batchingSettings.requestByteThreshold()))
.build();

batchingSettingsBuilderExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(batchingSettingsBuilderExpr)
.setMethodName("setDelayThreshold")
.setArguments(
createDurationOfMillisExpr(toValExpr(batchingSettings.delayThresholdMillis())))
.build();

// FlowControlSettings.
Expr flowControlSettingsExpr =
MethodInvocationExpr.builder()
.setStaticReferenceType(STATIC_TYPES.get("FlowControlSettings"))
.setMethodName("newBuilder")
.build();
if (batchingSettings.flowControlElementLimit() != null) {
flowControlSettingsExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(flowControlSettingsExpr)
.setMethodName("setMaxOutstandingElementCount")
.setArguments(toValExpr(batchingSettings.flowControlElementLimit()))
.build();
}
if (batchingSettings.flowControlByteLimit() != null) {
flowControlSettingsExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(flowControlSettingsExpr)
.setMethodName("setMaxOutstandingRequestBytes")
.setArguments(toValExpr(batchingSettings.flowControlByteLimit()))
.build();
}
flowControlSettingsExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(flowControlSettingsExpr)
.setMethodName("setLimitExceededBehavior")
.setArguments(
EnumRefExpr.builder()
.setType(STATIC_TYPES.get("LimitExceededBehavior"))
.setName(
JavaStyle.toUpperCamelCase(
batchingSettings
.flowControlLimitExceededBehavior()
.name()
.toLowerCase()))
.build())
.build();
flowControlSettingsExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(flowControlSettingsExpr)
.setMethodName("build")
.build();

batchingSettingsBuilderExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(batchingSettingsBuilderExpr)
.setMethodName("setFlowControlSettings")
.setArguments(flowControlSettingsExpr)
.build();

batchingSettingsBuilderExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(batchingSettingsBuilderExpr)
.setMethodName("build")
.build();

// Put everything together.
Expr builderExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(builderVarExpr)
.setMethodName(settingsGetterMethodName)
.build();
return MethodInvocationExpr.builder()
.setExprReferenceExpr(builderExpr)
.setMethodName("setBatchingSettings")
.setArguments(batchingSettingsBuilderExpr)
.build();
}

private static Expr createRetryCodeDefinitionExpr(
String codeName, List<Code> retryCodes, VariableExpr definitionsVarExpr) {
// Construct something like `definitions.put("code_name",
Expand Down Expand Up @@ -705,10 +599,7 @@ private static MethodInvocationExpr createDurationOfMillisExpr(ValueExpr valExpr
private static Map<String, TypeNode> createStaticTypes() {
List<Class> concreteClazzes =
Arrays.asList(
BatchingSettings.class,
org.threeten.bp.Duration.class,
FlowControlSettings.class,
FlowController.LimitExceededBehavior.class,
ImmutableMap.class,
ImmutableSet.class,
Lists.class,
Expand Down
134 changes: 14 additions & 120 deletions src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@
import com.google.api.core.ApiFunction;
import com.google.api.core.ApiFuture;
import com.google.api.core.BetaApi;
import com.google.api.gax.batching.BatchingSettings;
import com.google.api.gax.batching.FlowControlSettings;
import com.google.api.gax.batching.FlowController.LimitExceededBehavior;
import com.google.api.gax.batching.PartitionKey;
import com.google.api.gax.batching.RequestBuilder;
import com.google.api.gax.core.GaxProperties;
import com.google.api.gax.core.GoogleCredentialsProvider;
import com.google.api.gax.core.InstantiatingExecutorProvider;
Expand All @@ -35,9 +30,6 @@
import com.google.api.gax.retrying.RetrySettings;
import com.google.api.gax.rpc.ApiCallContext;
import com.google.api.gax.rpc.ApiClientHeaderProvider;
import com.google.api.gax.rpc.BatchedRequestIssuer;
import com.google.api.gax.rpc.BatchingCallSettings;
import com.google.api.gax.rpc.BatchingDescriptor;
import com.google.api.gax.rpc.ClientContext;
import com.google.api.gax.rpc.OperationCallSettings;
import com.google.api.gax.rpc.PageContext;
Expand Down Expand Up @@ -82,7 +74,6 @@
import com.google.api.generator.engine.ast.Variable;
import com.google.api.generator.engine.ast.VariableExpr;
import com.google.api.generator.gapic.model.Field;
import com.google.api.generator.gapic.model.GapicBatchingSettings;
import com.google.api.generator.gapic.model.GapicClass;
import com.google.api.generator.gapic.model.GapicServiceConfig;
import com.google.api.generator.gapic.model.Message;
Expand Down Expand Up @@ -221,12 +212,8 @@ private static Map<String, VariableExpr> createMethodSettingsClassMemberVarExprs
Map<String, VariableExpr> varExprs = new LinkedHashMap<>();

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

for (Method method : service.methods()) {
Optional<GapicBatchingSettings> batchingSettingOpt =
Objects.isNull(serviceConfig)
? Optional.empty()
: serviceConfig.getBatchingSetting(service, method);
if (batchingSettingOpt.isPresent()) {
statements.add(
exprToStatementFn.apply(
BatchingDescriptorComposer.createBatchingDescriptorFieldDeclExpr(
method, batchingSettingOpt.get(), messageTypes)));
}
statements.add(EMPTY_LINE_STATEMENT);
}

return statements;
}

Expand Down Expand Up @@ -1261,22 +1234,6 @@ private static MethodDefinition createNestedClassInitDefaultsMethod(
if (streamKind.equals(Method.Stream.CLIENT) || streamKind.equals(Method.Stream.BIDI)) {
continue;
}
if (!Objects.isNull(serviceConfig) && serviceConfig.hasBatchingSetting(service, method)) {
Optional<GapicBatchingSettings> batchingSettingOpt =
serviceConfig.getBatchingSetting(service, method);
Preconditions.checkState(
batchingSettingOpt.isPresent(),
String.format(
"No batching setting found for service %s, method %s",
service.name(), method.name()));
String settingsGetterMethodName =
String.format("%sSettings", JavaStyle.toLowerCamelCase(method.name()));
bodyStatements.add(
ExprStatement.withExpr(
RetrySettingsComposer.createBatchingBuilderSettingsExpr(
settingsGetterMethodName, batchingSettingOpt.get(), builderVarExpr)));
bodyStatements.add(EMPTY_LINE_STATEMENT);
}

bodyStatements.add(
ExprStatement.withExpr(
Expand Down Expand Up @@ -1352,8 +1309,6 @@ private static List<MethodDefinition> createNestedClassConstructorMethods(
.build());
Reference pagedSettingsBuilderRef =
ConcreteReference.withClazz(PagedCallSettings.Builder.class);
Reference batchingSettingsBuilderRef =
ConcreteReference.withClazz(BatchingCallSettings.Builder.class);
Reference unaryCallSettingsBuilderRef =
ConcreteReference.withClazz(UnaryCallSettings.Builder.class);
Function<TypeNode, Boolean> isUnaryCallSettingsBuilderFn =
Expand All @@ -1363,9 +1318,6 @@ private static List<MethodDefinition> createNestedClassConstructorMethods(
.equals(unaryCallSettingsBuilderRef);
Function<TypeNode, Boolean> isPagedCallSettingsBuilderFn =
t -> t.reference().copyAndSetGenerics(ImmutableList.of()).equals(pagedSettingsBuilderRef);
Function<TypeNode, Boolean> isBatchingCallSettingsBuilderFn =
t ->
t.reference().copyAndSetGenerics(ImmutableList.of()).equals(batchingSettingsBuilderRef);
Function<TypeNode, TypeNode> builderToCallSettingsFn =
t ->
TypeNode.withReference(
Expand Down Expand Up @@ -1396,61 +1348,20 @@ private static List<MethodDefinition> createNestedClassConstructorMethods(
String methodName = getMethodNameFromSettingsVarName(e.getKey());

if (!isPagedCallSettingsBuilderFn.apply(varType)) {
if (!isBatchingCallSettingsBuilderFn.apply(varType)) {
boolean isUnaryCallSettings = isUnaryCallSettingsBuilderFn.apply(varType);
Expr builderExpr =
AssignmentExpr.builder()
.setVariableExpr(varExpr)
.setValueExpr(
MethodInvocationExpr.builder()
.setStaticReferenceType(
builderToCallSettingsFn.apply(varExpr.type()))
.setMethodName(
isUnaryCallSettings
? "newUnaryCallSettingsBuilder"
: "newBuilder")
.setReturnType(varExpr.type())
.build())
.build();
return ExprStatement.withExpr(builderExpr);
}
Expr newBatchingSettingsExpr =
MethodInvocationExpr.builder()
.setStaticReferenceType(STATIC_TYPES.get("BatchingSettings"))
.setMethodName("newBuilder")
.build();
newBatchingSettingsExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(newBatchingSettingsExpr)
.setMethodName("build")
.build();

String batchingDescVarName =
String.format(
BATCHING_DESC_PATTERN, JavaStyle.toUpperSnakeCase(methodName));
Expr batchingSettingsBuilderExpr =
MethodInvocationExpr.builder()
.setStaticReferenceType(builderToCallSettingsFn.apply(varType))
.setMethodName("newBuilder")
.setArguments(
VariableExpr.withVariable(
Variable.builder()
.setType(STATIC_TYPES.get("BatchingDescriptor"))
.setName(batchingDescVarName)
.build()))
.build();
batchingSettingsBuilderExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(batchingSettingsBuilderExpr)
.setMethodName("setBatchingSettings")
.setArguments(newBatchingSettingsExpr)
.setReturnType(varType)
.build();

boolean isUnaryCallSettings = isUnaryCallSettingsBuilderFn.apply(varType);
Expr builderExpr =
AssignmentExpr.builder()
.setVariableExpr(varExpr)
.setValueExpr(batchingSettingsBuilderExpr)
.setValueExpr(
MethodInvocationExpr.builder()
.setStaticReferenceType(
builderToCallSettingsFn.apply(varExpr.type()))
.setMethodName(
isUnaryCallSettings
? "newUnaryCallSettingsBuilder"
: "newBuilder")
.setReturnType(varExpr.type())
.build())
.build();
return ExprStatement.withExpr(builderExpr);
}
Expand Down Expand Up @@ -1496,8 +1407,7 @@ private static List<MethodDefinition> createNestedClassConstructorMethods(
.filter(
v ->
isUnaryCallSettingsBuilderFn.apply(v.type())
|| isPagedCallSettingsBuilderFn.apply(v.type())
|| isBatchingCallSettingsBuilderFn.apply(v.type()))
|| isPagedCallSettingsBuilderFn.apply(v.type()))
.collect(Collectors.toList()))
.setReturnType(NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR.type())
.build())
Expand Down Expand Up @@ -1779,15 +1689,10 @@ private static Map<String, TypeNode> createStaticTypes() {
ApiClientHeaderProvider.class,
ApiFunction.class,
ApiFuture.class,
BatchedRequestIssuer.class,
BatchingCallSettings.class,
BatchingDescriptor.class,
BatchingSettings.class,
BetaApi.class,
ClientContext.class,
Duration.class,
Empty.class,
FlowControlSettings.class,
GaxGrpcProperties.class,
GaxProperties.class,
Generated.class,
Expand All @@ -1799,7 +1704,6 @@ private static Map<String, TypeNode> createStaticTypes() {
ImmutableSet.class,
InstantiatingExecutorProvider.class,
InstantiatingGrpcChannelProvider.class,
LimitExceededBehavior.class,
List.class,
Lists.class,
MonitoredResourceDescriptor.class,
Expand All @@ -1811,9 +1715,7 @@ private static Map<String, TypeNode> createStaticTypes() {
PagedCallSettings.class,
PagedListDescriptor.class,
PagedListResponseFactory.class,
PartitionKey.class,
ProtoOperationTransformers.class,
RequestBuilder.class,
RetrySettings.class,
ServerStreamingCallSettings.class,
StatusCode.class,
Expand Down Expand Up @@ -1969,10 +1871,7 @@ private static String getGrpcServiceStubTypeName(String serviceName) {
}

private static TypeNode getCallSettingsType(
Method method,
Map<String, TypeNode> types,
boolean isBatchingSettings,
final boolean isSettingsBuilder) {
Method method, Map<String, TypeNode> types, final boolean isSettingsBuilder) {
Function<Class, TypeNode> typeMakerFn =
clz -> TypeNode.withReference(ConcreteReference.withClazz(clz));
// Default: No streaming.
Expand All @@ -1982,11 +1881,6 @@ private static TypeNode getCallSettingsType(
isSettingsBuilder ? PagedCallSettings.Builder.class : PagedCallSettings.class)
: typeMakerFn.apply(
isSettingsBuilder ? UnaryCallSettings.Builder.class : UnaryCallSettings.class);
if (isBatchingSettings) {
callSettingsType =
typeMakerFn.apply(
isSettingsBuilder ? BatchingCallSettings.Builder.class : BatchingCallSettings.class);
}

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