Skip to content

Commit

Permalink
fix: DIREGAPIC LRO generated tests logic (#838)
Browse files Browse the repository at this point in the history
  • Loading branch information
vam-google authored Sep 9, 2021
1 parent 200fce7 commit 8ae8e6f
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import com.google.api.generator.gapic.model.Message;
import com.google.api.generator.gapic.model.Method;
import com.google.api.generator.gapic.model.MethodArgument;
import com.google.api.generator.gapic.model.OperationResponse;
import com.google.api.generator.gapic.model.ResourceName;
import com.google.api.generator.gapic.model.Service;
import com.google.api.generator.gapic.utils.JavaStyle;
Expand Down Expand Up @@ -413,7 +414,10 @@ private MethodDefinition createRpcTestMethod(
.setValueExpr(expectedResponseValExpr)
.build());

if (method.hasLro()) {
if (method.hasLro()
&& (method.lro().operationServiceStubType() == null
|| !method.lro().responseType().equals(method.outputType()))) {

VariableExpr resultOperationVarExpr =
VariableExpr.withVariable(
Variable.builder()
Expand Down Expand Up @@ -910,24 +914,6 @@ private void addDynamicTypes(GapicContext context, Service service, TypeStore ty
}
}

private static TypeNode getOperationCallSettingsTypeHelper(
Method protoMethod, boolean isBuilder) {
Preconditions.checkState(
protoMethod.hasLro(),
String.format("Cannot get OperationCallSettings on non-LRO method %s", protoMethod.name()));
Class callSettingsClazz =
isBuilder ? OperationCallSettings.Builder.class : OperationCallSettings.class;
return TypeNode.withReference(
ConcreteReference.builder()
.setClazz(callSettingsClazz)
.setGenerics(
Arrays.asList(
protoMethod.inputType().reference(),
protoMethod.lro().responseType().reference(),
protoMethod.lro().metadataType().reference()))
.build());
}

private static TypeNode getCallSettingsTypeHelper(
Method protoMethod, TypeStore typeStore, boolean isBuilder) {
Class callSettingsClazz = isBuilder ? UnaryCallSettings.Builder.class : UnaryCallSettings.class;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
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.resourcename.ResourceNameTokenizer;
Expand Down Expand Up @@ -265,9 +266,12 @@ public static Expr createSimpleMessageBuilderExpr(
.setStaticReferenceType(message.type())
.setMethodName("newBuilder")
.build();

for (Field field : message.fields()) {
if (field.isContainedInOneof() // Avoid colliding fields.
|| ((field.isMessage() || field.isEnum()) // Avoid importing unparsed messages.
|| ((field.isMessage()
|| (field.isEnum()
&& message.operationResponse() == null)) // Avoid importing unparsed messages.
&& !field.isRepeated()
&& !messageTypes.containsKey(field.type().reference().fullName()))) {
continue;
Expand All @@ -292,7 +296,34 @@ public static Expr createSimpleMessageBuilderExpr(
.setReturnType(TypeNode.STRING)
.build();
} else {
defaultExpr = createDefaultValue(field, true);
if (message.operationResponse() != null) {
if (field.name().equals(message.operationResponse().statusFieldName())) {
String statusTypeName = message.operationResponse().statusFieldTypeName();
String statusClassName = statusTypeName.substring(statusTypeName.lastIndexOf('.') + 1);

TypeNode statusType =
TypeNode.withReference(
VaporReference.builder()
.setName(statusClassName)
.setPakkage(message.type().reference().fullName())
.setIsStaticImport(false)
.build());
defaultExpr =
VariableExpr.builder()
.setVariable(Variable.builder().setName("DONE").setType(statusType).build())
.setStaticReferenceType(statusType)
.build();

} else if (field.name().equals(message.operationResponse().errorCodeFieldName())) {
defaultExpr =
ValueExpr.withValue(
PrimitiveValue.builder().setType(field.type()).setValue("0").build());
}
}

if (defaultExpr == null) {
defaultExpr = createDefaultValue(field, true);
}
}
builderExpr =
MethodInvocationExpr.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -511,11 +511,14 @@ private List<Expr> setOperationSnapshotFactoryExpr(
String statusTypeName = operationResponse.statusFieldTypeName();
String statusClassName = statusTypeName.substring(statusTypeName.lastIndexOf('.') + 1);

TypeNode opType =
protoMethod.hasLro() ? protoMethod.lro().responseType() : protoMethod.outputType();

TypeNode statusType =
TypeNode.withReference(
VaporReference.builder()
.setName(statusClassName)
.setPakkage(protoMethod.outputType().reference().fullName())
.setPakkage(opType.reference().fullName())
.setIsStaticImport(false)
.build());
VariableExpr statusDoneExpr =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public abstract class Message {

public abstract ImmutableMap<String, Field> fieldMap();

@Nullable
public abstract OperationResponse operationResponse();

public abstract Map<String, String> operationRequestFields();
Expand Down Expand Up @@ -113,8 +114,7 @@ public static Builder builder() {
.setFieldMap(Collections.emptyMap())
.setEnumValues(Collections.emptyMap())
.setOperationResponseFields(HashBiMap.create())
.setOperationRequestFields(Collections.emptyMap())
.setOperationResponse(OperationResponse.builder().build());
.setOperationRequestFields(Collections.emptyMap());
}

@AutoValue.Builder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,17 @@
package com.google.api.generator.gapic.model;

import com.google.auto.value.AutoValue;
import javax.annotation.Nullable;

@AutoValue
public abstract class OperationResponse {
@Nullable
public abstract String nameFieldName();

@Nullable
public abstract String statusFieldName();

@Nullable
public abstract String errorCodeFieldName();

@Nullable
public abstract String errorMessageFieldName();

@Nullable
public abstract String statusFieldTypeName();

public static Builder builder() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,7 @@ private static Map<String, Message> parseMessages(
List<FieldDescriptor> fields = messageDescriptor.getFields();
HashMap<String, String> operationRequestFields = new HashMap<String, String>();
BiMap<String, String> operationResponseFields = HashBiMap.create();
OperationResponse.Builder operationResponse = OperationResponse.builder();
OperationResponse.Builder operationResponse = null;
for (FieldDescriptor fd : fields) {
if (fd.getOptions().hasExtension(ExtendedOperationsProto.operationRequestField)) {
String orf = fd.getOptions().getExtension(ExtendedOperationsProto.operationRequestField);
Expand All @@ -577,6 +577,9 @@ private static Map<String, Message> parseMessages(
if (fd.getOptions().hasExtension(ExtendedOperationsProto.operationField)) {
OperationResponseMapping orm =
fd.getOptions().getExtension(ExtendedOperationsProto.operationField);
if (operationResponse == null) {
operationResponse = OperationResponse.builder();
}
if (orm.equals(OperationResponseMapping.NAME)) {
operationResponse.setNameFieldName(fd.getName());
} else if (orm.equals(OperationResponseMapping.STATUS)) {
Expand All @@ -599,7 +602,7 @@ private static Map<String, Message> parseMessages(
.setOuterNestedTypes(outerNestedTypes)
.setOperationRequestFields(operationRequestFields)
.setOperationResponseFields(operationResponseFields)
.setOperationResponse(operationResponse.build())
.setOperationResponse(operationResponse != null ? operationResponse.build() : null)
.build());
return messages;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,9 @@
import com.google.api.gax.rpc.InvalidArgumentException;
import com.google.api.gax.rpc.StatusCode;
import com.google.api.gax.rpc.testing.FakeStatusCode;
import com.google.cloud.compute.v1.Operation.Status;
import com.google.cloud.compute.v1.stub.HttpJsonAddressesStub;
import com.google.common.collect.Lists;
import com.google.longrunning.Operation;
import com.google.protobuf.Any;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -135,15 +134,15 @@ public void aggregatedListExceptionTest() throws Exception {

@Test
public void deleteTest() throws Exception {
com.google.cloud.compute.v1.Operation expectedResponse =
com.google.cloud.compute.v1.Operation.newBuilder()
Operation expectedResponse =
Operation.newBuilder()
.setClientOperationId("clientOperationId-1230366697")
.setCreationTimestamp("creationTimestamp-370203401")
.setDescription("description-1724546052")
.setEndTime("endTime-1607243192")
.setError(Error.newBuilder().build())
.setHttpErrorMessage("httpErrorMessage1577303431")
.setHttpErrorStatusCode(1386087020)
.setHttpErrorStatusCode(0)
.setId(3355)
.setInsertTime("insertTime966165798")
.setKind("kind3292052")
Expand All @@ -153,27 +152,21 @@ public void deleteTest() throws Exception {
.setRegion("region-934795532")
.setSelfLink("selfLink1191800166")
.setStartTime("startTime-2129294769")
.setStatus(Status.DONE)
.setStatusMessage("statusMessage-958704715")
.setTargetId(-815576439)
.setTargetLink("targetLink486368555")
.setUser("user3599307")
.addAllWarnings(new ArrayList<Warnings>())
.setZone("zone3744684")
.build();
Operation resultOperation =
Operation.newBuilder()
.setName("deleteTest")
.setDone(true)
.setResponse(Any.pack(expectedResponse))
.build();
mockService.addResponse(resultOperation);
mockService.addResponse(expectedResponse);

String project = "project-309310695";
String region = "region-934795532";
String address = "address-1147692044";

com.google.cloud.compute.v1.Operation actualResponse =
client.deleteAsync(project, region, address).get();
Operation actualResponse = client.deleteAsync(project, region, address).get();
Assert.assertEquals(expectedResponse, actualResponse);

List<String> actualRequests = mockService.getRequestPaths();
Expand Down Expand Up @@ -210,15 +203,15 @@ public void deleteExceptionTest() throws Exception {

@Test
public void insertTest() throws Exception {
com.google.cloud.compute.v1.Operation expectedResponse =
com.google.cloud.compute.v1.Operation.newBuilder()
Operation expectedResponse =
Operation.newBuilder()
.setClientOperationId("clientOperationId-1230366697")
.setCreationTimestamp("creationTimestamp-370203401")
.setDescription("description-1724546052")
.setEndTime("endTime-1607243192")
.setError(Error.newBuilder().build())
.setHttpErrorMessage("httpErrorMessage1577303431")
.setHttpErrorStatusCode(1386087020)
.setHttpErrorStatusCode(0)
.setId(3355)
.setInsertTime("insertTime966165798")
.setKind("kind3292052")
Expand All @@ -228,27 +221,21 @@ public void insertTest() throws Exception {
.setRegion("region-934795532")
.setSelfLink("selfLink1191800166")
.setStartTime("startTime-2129294769")
.setStatus(Status.DONE)
.setStatusMessage("statusMessage-958704715")
.setTargetId(-815576439)
.setTargetLink("targetLink486368555")
.setUser("user3599307")
.addAllWarnings(new ArrayList<Warnings>())
.setZone("zone3744684")
.build();
Operation resultOperation =
Operation.newBuilder()
.setName("insertTest")
.setDone(true)
.setResponse(Any.pack(expectedResponse))
.build();
mockService.addResponse(resultOperation);
mockService.addResponse(expectedResponse);

String project = "project-309310695";
String region = "region-934795532";
Address addressResource = Address.newBuilder().build();

com.google.cloud.compute.v1.Operation actualResponse =
client.insertAsync(project, region, addressResource).get();
Operation actualResponse = client.insertAsync(project, region, addressResource).get();
Assert.assertEquals(expectedResponse, actualResponse);

List<String> actualRequests = mockService.getRequestPaths();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.api.gax.rpc.InvalidArgumentException;
import com.google.api.gax.rpc.StatusCode;
import com.google.api.gax.rpc.testing.FakeStatusCode;
import com.google.cloud.compute.v1.Operation.Status;
import com.google.cloud.compute.v1.stub.HttpJsonRegionOperationsStub;
import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -82,7 +83,7 @@ public void getTest() throws Exception {
.setEndTime("endTime-1607243192")
.setError(Error.newBuilder().build())
.setHttpErrorMessage("httpErrorMessage1577303431")
.setHttpErrorStatusCode(1386087020)
.setHttpErrorStatusCode(0)
.setId(3355)
.setInsertTime("insertTime966165798")
.setKind("kind3292052")
Expand All @@ -92,6 +93,7 @@ public void getTest() throws Exception {
.setRegion("region-934795532")
.setSelfLink("selfLink1191800166")
.setStartTime("startTime-2129294769")
.setStatus(Status.DONE)
.setStatusMessage("statusMessage-958704715")
.setTargetId(-815576439)
.setTargetLink("targetLink486368555")
Expand Down Expand Up @@ -151,7 +153,7 @@ public void waitTest() throws Exception {
.setEndTime("endTime-1607243192")
.setError(Error.newBuilder().build())
.setHttpErrorMessage("httpErrorMessage1577303431")
.setHttpErrorStatusCode(1386087020)
.setHttpErrorStatusCode(0)
.setId(3355)
.setInsertTime("insertTime966165798")
.setKind("kind3292052")
Expand All @@ -161,6 +163,7 @@ public void waitTest() throws Exception {
.setRegion("region-934795532")
.setSelfLink("selfLink1191800166")
.setStartTime("startTime-2129294769")
.setStatus(Status.DONE)
.setStatusMessage("statusMessage-958704715")
.setTargetId(-815576439)
.setTargetLink("targetLink486368555")
Expand Down

0 comments on commit 8ae8e6f

Please sign in to comment.