Skip to content

feat: lro compute poc #14051

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

Draft
nnicolee wants to merge 2 commits into
base: feat/lro-generic-error-propagation
Choose a base branch
from
Draft

feat: lro compute poc #14051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ba039e9
feat: added poc for option c for compute lro error details
nnicolee Aug 12, 2026
f7641c5
added license headers for new files
nnicolee Aug 12, 2026
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
16 changes: 16 additions & 0 deletions ...enerator-java/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,11 +536,27 @@ public void visit(ArithmeticOperationExpr arithmeticOperationExpr) {

@Override
public void visit(UnaryOperationExpr unaryOperationExpr) {
boolean needsParen = unaryOperationExpr.expr() instanceof InstanceofExpr
|| unaryOperationExpr.expr() instanceof RelationalOperationExpr
|| unaryOperationExpr.expr() instanceof LogicalOperationExpr;
Comment on lines +539 to +541

@gemini-code-assist gemini-code-assist Bot Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current logic for needsParen only covers InstanceofExpr, RelationalOperationExpr, and LogicalOperationExpr. However, unary operators (such as logical negation ! or unary minus -) have higher precedence than most other expressions in Java.

To prevent operator precedence bugs or compilation errors, needsParen should also include other expression types that have lower precedence than unary operators, such as CastExpr (e.g., !((Boolean) x) vs !(Boolean) x which is a syntax error) and ArithmeticOperationExpr (e.g., -(a + b) vs -a + b which changes semantics).

    boolean needsParen = unaryOperationExpr.expr() instanceof InstanceofExpr
        || unaryOperationExpr.expr() instanceof RelationalOperationExpr
        || unaryOperationExpr.expr() instanceof LogicalOperationExpr
        || unaryOperationExpr.expr() instanceof CastExpr
        || unaryOperationExpr.expr() instanceof ArithmeticOperationExpr;


if (unaryOperationExpr.operatorKind().isPrefixOperator()) {
operator(unaryOperationExpr.operatorKind());
if (needsParen) {
leftParen();
}
unaryOperationExpr.expr().accept(this);
if (needsParen) {
rightParen();
}
} else {
if (needsParen) {
leftParen();
}
unaryOperationExpr.expr().accept(this);
if (needsParen) {
rightParen();
}
operator(unaryOperationExpr.operatorKind());
}
}
Expand Down
5 changes: 5 additions & 0 deletions .../gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/Composer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.api.generator.gapic.composer.grpc.ServiceStubSettingsClassComposer;
import com.google.api.generator.gapic.composer.grpcrest.HttpJsonServiceClientTestClassComposer;
import com.google.api.generator.gapic.composer.resourcename.ResourceNameHelperClassComposer;
import com.google.api.generator.gapic.composer.rest.ComputeLroErrorParserClassComposer;
import com.google.api.generator.gapic.composer.rest.HttpJsonServiceCallableFactoryClassComposer;
import com.google.api.generator.gapic.composer.rest.HttpJsonServiceStubClassComposer;
import com.google.api.generator.gapic.model.GapicClass;
Expand Down Expand Up @@ -102,6 +103,10 @@ public static List<GapicClass> generateStubClasses(GapicContext context) {
clazzes.add(
HttpJsonServiceCallableFactoryClassComposer.instance().generate(context, s));
clazzes.add(HttpJsonServiceStubClassComposer.instance().generate(context, s));
if (s.pakkage().startsWith("com.google.cloud.compute.v1")) {
clazzes.add(
ComputeLroErrorParserClassComposer.instance().generate(context, s));
}
Comment on lines +106 to +109

@gemini-code-assist gemini-code-assist Bot Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Hardcoding the package prefix "com.google.cloud.compute.v1" directly in the core Composer class introduces tight coupling to a specific service (Compute) and violates the Open-Closed Principle.

If other services in the future require custom LRO error parsing, this class will have to be modified again. Consider making this behavior configurable or metadata-driven (e.g., via a flag in GapicContext or service configuration) rather than hardcoding package names.

} else if (context.transport() == Transport.GRPC) {
clazzes.add(ServiceStubClassComposer.instance().generate(context, s));
clazzes.add(ServiceStubSettingsClassComposer.instance().generate(context, s));
Expand Down
19 changes: 18 additions & 1 deletion ...a/src/main/java/com/google/api/generator/gapic/composer/common/RetrySettingsComposer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,12 @@
import com.google.api.generator.engine.ast.Expr;
import com.google.api.generator.engine.ast.ExprStatement;
import com.google.api.generator.engine.ast.MethodInvocationExpr;
import com.google.api.generator.engine.ast.NewObjectExpr;
import com.google.api.generator.engine.ast.PrimitiveValue;
import com.google.api.generator.engine.ast.StringObjectValue;
import com.google.api.generator.engine.ast.TypeNode;
import com.google.api.generator.engine.ast.ValueExpr;
import com.google.api.generator.engine.ast.VaporReference;
import com.google.api.generator.engine.ast.Variable;
import com.google.api.generator.engine.ast.VariableExpr;
import com.google.api.generator.gapic.composer.store.TypeStore;
Expand Down Expand Up @@ -323,6 +325,21 @@ public static Expr createLroSettingsBuilderExpr(
Variable.builder().setType(TypeNode.CLASS_OBJECT).setName("class").build())
.setStaticReferenceType(t)
.build();

List<Expr> createArgs = new ArrayList<>();
createArgs.add(classFieldRefFn.apply(method.lro().responseType()));
if (service.pakkage().startsWith("com.google.cloud.compute.v1") && operationResponseTransformer.reference().pakkage().equals("com.google.api.gax.httpjson")) {
createArgs.add(
NewObjectExpr.builder()
.setType(
TypeNode.withReference(
VaporReference.builder()
.setName("ComputeLroErrorParser")
.setPakkage(service.pakkage() + ".stub")
.build()))
.build());
}
Comment on lines +331 to +341

@gemini-code-assist gemini-code-assist Bot Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to the change in Composer.java, hardcoding the package prefix "com.google.cloud.compute.v1" here introduces tight coupling to the Compute service.

Consider abstracting this check by introducing a property on the Service or GapicContext model (e.g., service.hasCustomLroErrorParser() or similar) to determine if a custom LRO error parser should be instantiated and passed to the response transformer.


builderSettingsExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(builderSettingsExpr)
Expand All @@ -331,7 +348,7 @@ public static Expr createLroSettingsBuilderExpr(
MethodInvocationExpr.builder()
.setStaticReferenceType(operationResponseTransformer)
.setMethodName("create")
.setArguments(classFieldRefFn.apply(method.lro().responseType()))
.setArguments(createArgs)
.build())
.build();
builderSettingsExpr =
Expand Down
229 changes: 229 additions & 0 deletions ...java/com/google/api/generator/gapic/composer/rest/ComputeLroErrorParserClassComposer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,229 @@
// Copyright 2026 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

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

import com.google.api.generator.engine.ast.AssignmentExpr;
import com.google.api.generator.engine.ast.BlockStatement;
import com.google.api.generator.engine.ast.CastExpr;
import com.google.api.generator.engine.ast.ClassDefinition;
import com.google.api.generator.engine.ast.ConcreteReference;
import com.google.api.generator.engine.ast.Expr;
import com.google.api.generator.engine.ast.ExprStatement;
import com.google.api.generator.engine.ast.ForStatement;
import com.google.api.generator.engine.ast.IfStatement;
import com.google.api.generator.engine.ast.InstanceofExpr;
import com.google.api.generator.engine.ast.LogicalOperationExpr;
import com.google.api.generator.engine.ast.UnaryOperationExpr;
import com.google.api.generator.engine.ast.MethodDefinition;
import com.google.api.generator.engine.ast.MethodInvocationExpr;
import com.google.api.generator.engine.ast.NewObjectExpr;
import com.google.api.generator.engine.ast.PrimitiveValue;
import com.google.api.generator.engine.ast.RelationalOperationExpr;
import com.google.api.generator.engine.ast.ReturnExpr;
import com.google.api.generator.engine.ast.ScopeNode;
import com.google.api.generator.engine.ast.Statement;
import com.google.api.generator.engine.ast.StringObjectValue;
import com.google.api.generator.engine.ast.TypeNode;
import com.google.api.generator.engine.ast.ValueExpr;
import com.google.api.generator.engine.ast.VaporReference;
import com.google.api.generator.engine.ast.Variable;
import com.google.api.generator.engine.ast.VariableExpr;
import com.google.api.generator.gapic.composer.comment.CommentComposer;
import com.google.api.generator.gapic.model.GapicClass;
import com.google.api.generator.gapic.model.GapicContext;
import com.google.api.generator.gapic.model.Service;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

public class ComputeLroErrorParserClassComposer {
private static final ComputeLroErrorParserClassComposer INSTANCE = new ComputeLroErrorParserClassComposer();

public static ComputeLroErrorParserClassComposer instance() {
return INSTANCE;
}

public GapicClass generate(GapicContext context, Service service) {
String pakkage = service.pakkage() + ".stub";
TypeNode parserType = TypeNode.withReference(
VaporReference.builder().setName("ComputeLroErrorParser").setPakkage(pakkage).build());

TypeNode httpJsonLroErrorParserType = TypeNode.withReference(
ConcreteReference.withClazz(com.google.api.gax.httpjson.HttpJsonLroErrorParser.class));
TypeNode errorDetailsType = TypeNode.withReference(
ConcreteReference.withClazz(com.google.api.gax.rpc.ErrorDetails.class));
TypeNode operationType = TypeNode.withReference(
VaporReference.builder().setName("Operation").setPakkage(service.pakkage()).build());
TypeNode errorsType = TypeNode.withReference(
VaporReference.builder().setName("Errors").setPakkage(service.pakkage()).build());
TypeNode errorInfoType = TypeNode.withReference(
ConcreteReference.withClazz(com.google.rpc.ErrorInfo.class));
TypeNode anyType = TypeNode.withReference(
ConcreteReference.withClazz(com.google.protobuf.Any.class));
TypeNode listAnyType = TypeNode.withReference(
ConcreteReference.builder().setClazz(java.util.List.class).setGenerics(anyType.reference()).build());
TypeNode arrayListAnyType = TypeNode.withReference(
ConcreteReference.builder().setClazz(java.util.ArrayList.class).setGenerics(anyType.reference()).build());

VariableExpr responseVar = VariableExpr.withVariable(
Variable.builder().setType(TypeNode.OBJECT).setName("response").build());
VariableExpr operationVar = VariableExpr.withVariable(
Variable.builder().setType(operationType).setName("operation").build());
VariableExpr rawErrorMessagesVar = VariableExpr.withVariable(
Variable.builder().setType(listAnyType).setName("rawErrorMessages").build());
VariableExpr errorVar = VariableExpr.withVariable(
Variable.builder().setType(errorsType).setName("error").build());
VariableExpr errorInfoVar = VariableExpr.withVariable(
Variable.builder().setType(errorInfoType).setName("errorInfo").build());
VariableExpr sbVar = VariableExpr.withVariable(
Variable.builder().setType(TypeNode.withReference(ConcreteReference.withClazz(StringBuilder.class))).setName("sb").build());

// parse() method
List<Statement> parseBody = new ArrayList<>();
parseBody.add(IfStatement.builder()
.setConditionExpr(UnaryOperationExpr.logicalNotWithExpr(
InstanceofExpr.builder().setExpr(responseVar).setCheckType(operationType).build()))
.setBody(Arrays.asList(ExprStatement.withExpr(ReturnExpr.withExpr(ValueExpr.createNullExpr()))))
.build());
parseBody.add(ExprStatement.withExpr(AssignmentExpr.builder()
.setVariableExpr(operationVar.toBuilder().setIsDecl(true).build())
.setValueExpr(CastExpr.builder().setType(operationType).setExpr(responseVar).build())
.build()));
parseBody.add(IfStatement.builder()
.setConditionExpr(UnaryOperationExpr.logicalNotWithExpr(
MethodInvocationExpr.builder().setExprReferenceExpr(operationVar).setMethodName("hasError").setReturnType(TypeNode.BOOLEAN).build()))
.setBody(Arrays.asList(ExprStatement.withExpr(ReturnExpr.withExpr(ValueExpr.createNullExpr()))))
.build());
parseBody.add(ExprStatement.withExpr(AssignmentExpr.builder()
.setVariableExpr(rawErrorMessagesVar.toBuilder().setIsDecl(true).build())
.setValueExpr(NewObjectExpr.builder().setType(arrayListAnyType).build())
.build()));

Expr getErrorsListExpr = MethodInvocationExpr.builder()
.setExprReferenceExpr(MethodInvocationExpr.builder().setExprReferenceExpr(operationVar).setMethodName("getError").build())
.setMethodName("getErrorsList").build();

List<Statement> forBody = new ArrayList<>();
Expr errorInfoBuilderExpr = MethodInvocationExpr.builder().setStaticReferenceType(errorInfoType).setMethodName("newBuilder").build();
errorInfoBuilderExpr = MethodInvocationExpr.builder().setExprReferenceExpr(errorInfoBuilderExpr).setMethodName("setReason")
.setArguments(MethodInvocationExpr.builder().setExprReferenceExpr(errorVar).setMethodName("getCode").setReturnType(TypeNode.STRING).build()).build();
errorInfoBuilderExpr = MethodInvocationExpr.builder().setExprReferenceExpr(errorInfoBuilderExpr).setMethodName("setDomain")
.setArguments(ValueExpr.withValue(StringObjectValue.withValue("googleapis.com"))).build();
errorInfoBuilderExpr = MethodInvocationExpr.builder().setExprReferenceExpr(errorInfoBuilderExpr).setMethodName("putMetadata")
.setArguments(ValueExpr.withValue(StringObjectValue.withValue("message")), MethodInvocationExpr.builder().setExprReferenceExpr(errorVar).setMethodName("getMessage").setReturnType(TypeNode.STRING).build()).build();
errorInfoBuilderExpr = MethodInvocationExpr.builder().setExprReferenceExpr(errorInfoBuilderExpr).setMethodName("putMetadata")
.setArguments(ValueExpr.withValue(StringObjectValue.withValue("location")), MethodInvocationExpr.builder().setExprReferenceExpr(errorVar).setMethodName("getLocation").setReturnType(TypeNode.STRING).build()).build();
errorInfoBuilderExpr = MethodInvocationExpr.builder().setExprReferenceExpr(errorInfoBuilderExpr).setMethodName("build").setReturnType(errorInfoType).build();

forBody.add(ExprStatement.withExpr(AssignmentExpr.builder()
.setVariableExpr(errorInfoVar.toBuilder().setIsDecl(true).build())
.setValueExpr(errorInfoBuilderExpr)
.build()));
forBody.add(ExprStatement.withExpr(MethodInvocationExpr.builder()
.setExprReferenceExpr(rawErrorMessagesVar).setMethodName("add")
.setArguments(MethodInvocationExpr.builder().setStaticReferenceType(anyType).setMethodName("pack").setArguments(errorInfoVar).setReturnType(anyType).build())
.build()));

parseBody.add(ForStatement.builder()
.setLocalVariableExpr(errorVar.toBuilder().setIsDecl(true).build())
.setCollectionExpr(getErrorsListExpr)
.setBody(forBody)
.build());

Expr parseReturnExpr = MethodInvocationExpr.builder()
.setExprReferenceExpr(MethodInvocationExpr.builder()
.setExprReferenceExpr(MethodInvocationExpr.builder().setStaticReferenceType(errorDetailsType).setMethodName("builder").build())
.setMethodName("setRawErrorMessages").setArguments(rawErrorMessagesVar).build())
.setMethodName("build").setReturnType(errorDetailsType).build();

MethodDefinition parseMethod = MethodDefinition.builder()
.setAnnotations(Arrays.asList(com.google.api.generator.engine.ast.AnnotationNode.OVERRIDE))
.setScope(ScopeNode.PUBLIC)
.setReturnType(errorDetailsType)
.setName("parse")
.setArguments(responseVar.toBuilder().setIsDecl(true).build())
.setBody(parseBody)
.setReturnExpr(parseReturnExpr)
.build();

// parseErrorMessage() method
List<Statement> parseErrorBody = new ArrayList<>();
parseErrorBody.add(IfStatement.builder()
.setConditionExpr(UnaryOperationExpr.logicalNotWithExpr(
InstanceofExpr.builder().setExpr(responseVar).setCheckType(operationType).build()))
.setBody(Arrays.asList(ExprStatement.withExpr(ReturnExpr.withExpr(ValueExpr.createNullExpr()))))
.build());
parseErrorBody.add(ExprStatement.withExpr(AssignmentExpr.builder()
.setVariableExpr(operationVar.toBuilder().setIsDecl(true).build())
.setValueExpr(CastExpr.builder().setType(operationType).setExpr(responseVar).build())
.build()));

Expr condition1 = UnaryOperationExpr.logicalNotWithExpr(MethodInvocationExpr.builder().setExprReferenceExpr(operationVar).setMethodName("hasError").setReturnType(TypeNode.BOOLEAN).build());
Expr condition2 = RelationalOperationExpr.equalToWithExprs(
MethodInvocationExpr.builder().setExprReferenceExpr(MethodInvocationExpr.builder().setExprReferenceExpr(operationVar).setMethodName("getError").build()).setMethodName("getErrorsCount").setReturnType(TypeNode.INT).build(),
ValueExpr.withValue(PrimitiveValue.builder().setType(TypeNode.INT).setValue("0").build()));
parseErrorBody.add(IfStatement.builder()
.setConditionExpr(LogicalOperationExpr.logicalOrWithExprs(condition1, condition2))
.setBody(Arrays.asList(ExprStatement.withExpr(ReturnExpr.withExpr(ValueExpr.createNullExpr()))))
.build());

parseErrorBody.add(ExprStatement.withExpr(AssignmentExpr.builder()
.setVariableExpr(sbVar.toBuilder().setIsDecl(true).build())
.setValueExpr(NewObjectExpr.builder().setType(sbVar.type()).build())
.build()));

List<Statement> forBody2 = new ArrayList<>();
forBody2.add(IfStatement.builder()
.setConditionExpr(RelationalOperationExpr.lessThanWithExprs(
ValueExpr.withValue(PrimitiveValue.builder().setType(TypeNode.INT).setValue("0").build()),
MethodInvocationExpr.builder().setExprReferenceExpr(sbVar).setMethodName("length").setReturnType(TypeNode.INT).build()))
.setBody(Arrays.asList(ExprStatement.withExpr(MethodInvocationExpr.builder().setExprReferenceExpr(sbVar).setMethodName("append").setArguments(ValueExpr.withValue(StringObjectValue.withValue("; "))).build())))
.build());

Expr appendExpr = MethodInvocationExpr.builder().setExprReferenceExpr(sbVar).setMethodName("append").setArguments(MethodInvocationExpr.builder().setExprReferenceExpr(errorVar).setMethodName("getCode").build()).build();
appendExpr = MethodInvocationExpr.builder().setExprReferenceExpr(appendExpr).setMethodName("append").setArguments(ValueExpr.withValue(StringObjectValue.withValue(": "))).build();
appendExpr = MethodInvocationExpr.builder().setExprReferenceExpr(appendExpr).setMethodName("append").setArguments(MethodInvocationExpr.builder().setExprReferenceExpr(errorVar).setMethodName("getMessage").build()).build();
forBody2.add(ExprStatement.withExpr(appendExpr));

parseErrorBody.add(ForStatement.builder()
.setLocalVariableExpr(errorVar.toBuilder().setIsDecl(true).build())
.setCollectionExpr(getErrorsListExpr)
.setBody(forBody2)
.build());

Expr parseErrorReturnExpr = MethodInvocationExpr.builder().setExprReferenceExpr(sbVar).setMethodName("toString").setReturnType(TypeNode.STRING).build();

MethodDefinition parseErrorMethod = MethodDefinition.builder()
.setAnnotations(Arrays.asList(com.google.api.generator.engine.ast.AnnotationNode.OVERRIDE))
.setScope(ScopeNode.PUBLIC)
.setReturnType(TypeNode.STRING)
.setName("parseErrorMessage")
.setArguments(responseVar.toBuilder().setIsDecl(true).build())
.setBody(parseErrorBody)
.setReturnExpr(parseErrorReturnExpr)
.build();

ClassDefinition classDef = ClassDefinition.builder()
.setFileHeader(CommentComposer.APACHE_LICENSE_COMMENT)
.setPackageString(pakkage)
.setName("ComputeLroErrorParser")
.setScope(ScopeNode.PUBLIC)
.setImplementsTypes(Arrays.asList(httpJsonLroErrorParserType))
.setMethods(Arrays.asList(parseMethod, parseErrorMethod))
.build();

return GapicClass.create(GapicClass.Kind.STUB, classDef);
}
}
Loading
Loading