Skip to content

Commit 8ae8e6f

Browse files
vam-googlevam-google
authored
fix: DIREGAPIC LRO generated tests logic (#838)
1 parent 200fce7 commit 8ae8e6f

File tree

8 files changed

+67
-60
lines changed

8 files changed

+67
-60
lines changed

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

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
import com.google.api.generator.gapic.model.Message;
6060
import com.google.api.generator.gapic.model.Method;
6161
import com.google.api.generator.gapic.model.MethodArgument;
62+
import com.google.api.generator.gapic.model.OperationResponse;
6263
import com.google.api.generator.gapic.model.ResourceName;
6364
import com.google.api.generator.gapic.model.Service;
6465
import com.google.api.generator.gapic.utils.JavaStyle;
@@ -413,7 +414,10 @@ private MethodDefinition createRpcTestMethod(
413414
.setValueExpr(expectedResponseValExpr)
414415
.build());
415416

416-
if (method.hasLro()) {
417+
if (method.hasLro()
418+
&& (method.lro().operationServiceStubType() == null
419+
|| !method.lro().responseType().equals(method.outputType()))) {
420+
417421
VariableExpr resultOperationVarExpr =
418422
VariableExpr.withVariable(
419423
Variable.builder()
@@ -910,24 +914,6 @@ private void addDynamicTypes(GapicContext context, Service service, TypeStore ty
910914
}
911915
}
912916

913-
private static TypeNode getOperationCallSettingsTypeHelper(
914-
Method protoMethod, boolean isBuilder) {
915-
Preconditions.checkState(
916-
protoMethod.hasLro(),
917-
String.format("Cannot get OperationCallSettings on non-LRO method %s", protoMethod.name()));
918-
Class callSettingsClazz =
919-
isBuilder ? OperationCallSettings.Builder.class : OperationCallSettings.class;
920-
return TypeNode.withReference(
921-
ConcreteReference.builder()
922-
.setClazz(callSettingsClazz)
923-
.setGenerics(
924-
Arrays.asList(
925-
protoMethod.inputType().reference(),
926-
protoMethod.lro().responseType().reference(),
927-
protoMethod.lro().metadataType().reference()))
928-
.build());
929-
}
930-
931917
private static TypeNode getCallSettingsTypeHelper(
932918
Method protoMethod, TypeStore typeStore, boolean isBuilder) {
933919
Class callSettingsClazz = isBuilder ? UnaryCallSettings.Builder.class : UnaryCallSettings.class;

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

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import com.google.api.generator.engine.ast.StringObjectValue;
2828
import com.google.api.generator.engine.ast.TypeNode;
2929
import com.google.api.generator.engine.ast.ValueExpr;
30+
import com.google.api.generator.engine.ast.VaporReference;
3031
import com.google.api.generator.engine.ast.Variable;
3132
import com.google.api.generator.engine.ast.VariableExpr;
3233
import com.google.api.generator.gapic.composer.resourcename.ResourceNameTokenizer;
@@ -265,9 +266,12 @@ public static Expr createSimpleMessageBuilderExpr(
265266
.setStaticReferenceType(message.type())
266267
.setMethodName("newBuilder")
267268
.build();
269+
268270
for (Field field : message.fields()) {
269271
if (field.isContainedInOneof() // Avoid colliding fields.
270-
|| ((field.isMessage() || field.isEnum()) // Avoid importing unparsed messages.
272+
|| ((field.isMessage()
273+
|| (field.isEnum()
274+
&& message.operationResponse() == null)) // Avoid importing unparsed messages.
271275
&& !field.isRepeated()
272276
&& !messageTypes.containsKey(field.type().reference().fullName()))) {
273277
continue;
@@ -292,7 +296,34 @@ public static Expr createSimpleMessageBuilderExpr(
292296
.setReturnType(TypeNode.STRING)
293297
.build();
294298
} else {
295-
defaultExpr = createDefaultValue(field, true);
299+
if (message.operationResponse() != null) {
300+
if (field.name().equals(message.operationResponse().statusFieldName())) {
301+
String statusTypeName = message.operationResponse().statusFieldTypeName();
302+
String statusClassName = statusTypeName.substring(statusTypeName.lastIndexOf('.') + 1);
303+
304+
TypeNode statusType =
305+
TypeNode.withReference(
306+
VaporReference.builder()
307+
.setName(statusClassName)
308+
.setPakkage(message.type().reference().fullName())
309+
.setIsStaticImport(false)
310+
.build());
311+
defaultExpr =
312+
VariableExpr.builder()
313+
.setVariable(Variable.builder().setName("DONE").setType(statusType).build())
314+
.setStaticReferenceType(statusType)
315+
.build();
316+
317+
} else if (field.name().equals(message.operationResponse().errorCodeFieldName())) {
318+
defaultExpr =
319+
ValueExpr.withValue(
320+
PrimitiveValue.builder().setType(field.type()).setValue("0").build());
321+
}
322+
}
323+
324+
if (defaultExpr == null) {
325+
defaultExpr = createDefaultValue(field, true);
326+
}
296327
}
297328
builderExpr =
298329
MethodInvocationExpr.builder()

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -511,11 +511,14 @@ private List<Expr> setOperationSnapshotFactoryExpr(
511511
String statusTypeName = operationResponse.statusFieldTypeName();
512512
String statusClassName = statusTypeName.substring(statusTypeName.lastIndexOf('.') + 1);
513513

514+
TypeNode opType =
515+
protoMethod.hasLro() ? protoMethod.lro().responseType() : protoMethod.outputType();
516+
514517
TypeNode statusType =
515518
TypeNode.withReference(
516519
VaporReference.builder()
517520
.setName(statusClassName)
518-
.setPakkage(protoMethod.outputType().reference().fullName())
521+
.setPakkage(opType.reference().fullName())
519522
.setIsStaticImport(false)
520523
.build());
521524
VariableExpr statusDoneExpr =

‎src/main/java/com/google/api/generator/gapic/model/Message.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public abstract class Message {
5353

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

56+
@Nullable
5657
public abstract OperationResponse operationResponse();
5758

5859
public abstract Map<String, String> operationRequestFields();
@@ -113,8 +114,7 @@ public static Builder builder() {
113114
.setFieldMap(Collections.emptyMap())
114115
.setEnumValues(Collections.emptyMap())
115116
.setOperationResponseFields(HashBiMap.create())
116-
.setOperationRequestFields(Collections.emptyMap())
117-
.setOperationResponse(OperationResponse.builder().build());
117+
.setOperationRequestFields(Collections.emptyMap());
118118
}
119119

120120
@AutoValue.Builder

‎src/main/java/com/google/api/generator/gapic/model/OperationResponse.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,17 @@
1616
package com.google.api.generator.gapic.model;
1717

1818
import com.google.auto.value.AutoValue;
19-
import javax.annotation.Nullable;
2019

2120
@AutoValue
2221
public abstract class OperationResponse {
23-
@Nullable
2422
public abstract String nameFieldName();
2523

26-
@Nullable
2724
public abstract String statusFieldName();
2825

29-
@Nullable
3026
public abstract String errorCodeFieldName();
3127

32-
@Nullable
3328
public abstract String errorMessageFieldName();
3429

35-
@Nullable
3630
public abstract String statusFieldTypeName();
3731

3832
public static Builder builder() {

‎src/main/java/com/google/api/generator/gapic/protoparser/Parser.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,7 @@ private static Map<String, Message> parseMessages(
564564
List<FieldDescriptor> fields = messageDescriptor.getFields();
565565
HashMap<String, String> operationRequestFields = new HashMap<String, String>();
566566
BiMap<String, String> operationResponseFields = HashBiMap.create();
567-
OperationResponse.Builder operationResponse = OperationResponse.builder();
567+
OperationResponse.Builder operationResponse = null;
568568
for (FieldDescriptor fd : fields) {
569569
if (fd.getOptions().hasExtension(ExtendedOperationsProto.operationRequestField)) {
570570
String orf = fd.getOptions().getExtension(ExtendedOperationsProto.operationRequestField);
@@ -577,6 +577,9 @@ private static Map<String, Message> parseMessages(
577577
if (fd.getOptions().hasExtension(ExtendedOperationsProto.operationField)) {
578578
OperationResponseMapping orm =
579579
fd.getOptions().getExtension(ExtendedOperationsProto.operationField);
580+
if (operationResponse == null) {
581+
operationResponse = OperationResponse.builder();
582+
}
580583
if (orm.equals(OperationResponseMapping.NAME)) {
581584
operationResponse.setNameFieldName(fd.getName());
582585
} else if (orm.equals(OperationResponseMapping.STATUS)) {
@@ -599,7 +602,7 @@ private static Map<String, Message> parseMessages(
599602
.setOuterNestedTypes(outerNestedTypes)
600603
.setOperationRequestFields(operationRequestFields)
601604
.setOperationResponseFields(operationResponseFields)
602-
.setOperationResponse(operationResponse.build())
605+
.setOperationResponse(operationResponse != null ? operationResponse.build() : null)
603606
.build());
604607
return messages;
605608
}

‎test/integration/goldens/compute/com/google/cloud/compute/v1/AddressesClientTest.java‎

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,9 @@
2828
import com.google.api.gax.rpc.InvalidArgumentException;
2929
import com.google.api.gax.rpc.StatusCode;
3030
import com.google.api.gax.rpc.testing.FakeStatusCode;
31+
import com.google.cloud.compute.v1.Operation.Status;
3132
import com.google.cloud.compute.v1.stub.HttpJsonAddressesStub;
3233
import com.google.common.collect.Lists;
33-
import com.google.longrunning.Operation;
34-
import com.google.protobuf.Any;
3534
import java.io.IOException;
3635
import java.util.ArrayList;
3736
import java.util.Arrays;
@@ -135,15 +134,15 @@ public void aggregatedListExceptionTest() throws Exception {
135134

136135
@Test
137136
public void deleteTest() throws Exception {
138-
com.google.cloud.compute.v1.Operation expectedResponse =
139-
com.google.cloud.compute.v1.Operation.newBuilder()
137+
Operation expectedResponse =
138+
Operation.newBuilder()
140139
.setClientOperationId("clientOperationId-1230366697")
141140
.setCreationTimestamp("creationTimestamp-370203401")
142141
.setDescription("description-1724546052")
143142
.setEndTime("endTime-1607243192")
144143
.setError(Error.newBuilder().build())
145144
.setHttpErrorMessage("httpErrorMessage1577303431")
146-
.setHttpErrorStatusCode(1386087020)
145+
.setHttpErrorStatusCode(0)
147146
.setId(3355)
148147
.setInsertTime("insertTime966165798")
149148
.setKind("kind3292052")
@@ -153,27 +152,21 @@ public void deleteTest() throws Exception {
153152
.setRegion("region-934795532")
154153
.setSelfLink("selfLink1191800166")
155154
.setStartTime("startTime-2129294769")
155+
.setStatus(Status.DONE)
156156
.setStatusMessage("statusMessage-958704715")
157157
.setTargetId(-815576439)
158158
.setTargetLink("targetLink486368555")
159159
.setUser("user3599307")
160160
.addAllWarnings(new ArrayList<Warnings>())
161161
.setZone("zone3744684")
162162
.build();
163-
Operation resultOperation =
164-
Operation.newBuilder()
165-
.setName("deleteTest")
166-
.setDone(true)
167-
.setResponse(Any.pack(expectedResponse))
168-
.build();
169-
mockService.addResponse(resultOperation);
163+
mockService.addResponse(expectedResponse);
170164

171165
String project = "project-309310695";
172166
String region = "region-934795532";
173167
String address = "address-1147692044";
174168

175-
com.google.cloud.compute.v1.Operation actualResponse =
176-
client.deleteAsync(project, region, address).get();
169+
Operation actualResponse = client.deleteAsync(project, region, address).get();
177170
Assert.assertEquals(expectedResponse, actualResponse);
178171

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

211204
@Test
212205
public void insertTest() throws Exception {
213-
com.google.cloud.compute.v1.Operation expectedResponse =
214-
com.google.cloud.compute.v1.Operation.newBuilder()
206+
Operation expectedResponse =
207+
Operation.newBuilder()
215208
.setClientOperationId("clientOperationId-1230366697")
216209
.setCreationTimestamp("creationTimestamp-370203401")
217210
.setDescription("description-1724546052")
218211
.setEndTime("endTime-1607243192")
219212
.setError(Error.newBuilder().build())
220213
.setHttpErrorMessage("httpErrorMessage1577303431")
221-
.setHttpErrorStatusCode(1386087020)
214+
.setHttpErrorStatusCode(0)
222215
.setId(3355)
223216
.setInsertTime("insertTime966165798")
224217
.setKind("kind3292052")
@@ -228,27 +221,21 @@ public void insertTest() throws Exception {
228221
.setRegion("region-934795532")
229222
.setSelfLink("selfLink1191800166")
230223
.setStartTime("startTime-2129294769")
224+
.setStatus(Status.DONE)
231225
.setStatusMessage("statusMessage-958704715")
232226
.setTargetId(-815576439)
233227
.setTargetLink("targetLink486368555")
234228
.setUser("user3599307")
235229
.addAllWarnings(new ArrayList<Warnings>())
236230
.setZone("zone3744684")
237231
.build();
238-
Operation resultOperation =
239-
Operation.newBuilder()
240-
.setName("insertTest")
241-
.setDone(true)
242-
.setResponse(Any.pack(expectedResponse))
243-
.build();
244-
mockService.addResponse(resultOperation);
232+
mockService.addResponse(expectedResponse);
245233

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

250-
com.google.cloud.compute.v1.Operation actualResponse =
251-
client.insertAsync(project, region, addressResource).get();
238+
Operation actualResponse = client.insertAsync(project, region, addressResource).get();
252239
Assert.assertEquals(expectedResponse, actualResponse);
253240

254241
List<String> actualRequests = mockService.getRequestPaths();

‎test/integration/goldens/compute/com/google/cloud/compute/v1/RegionOperationsClientTest.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.google.api.gax.rpc.InvalidArgumentException;
2626
import com.google.api.gax.rpc.StatusCode;
2727
import com.google.api.gax.rpc.testing.FakeStatusCode;
28+
import com.google.cloud.compute.v1.Operation.Status;
2829
import com.google.cloud.compute.v1.stub.HttpJsonRegionOperationsStub;
2930
import java.io.IOException;
3031
import java.util.ArrayList;
@@ -82,7 +83,7 @@ public void getTest() throws Exception {
8283
.setEndTime("endTime-1607243192")
8384
.setError(Error.newBuilder().build())
8485
.setHttpErrorMessage("httpErrorMessage1577303431")
85-
.setHttpErrorStatusCode(1386087020)
86+
.setHttpErrorStatusCode(0)
8687
.setId(3355)
8788
.setInsertTime("insertTime966165798")
8889
.setKind("kind3292052")
@@ -92,6 +93,7 @@ public void getTest() throws Exception {
9293
.setRegion("region-934795532")
9394
.setSelfLink("selfLink1191800166")
9495
.setStartTime("startTime-2129294769")
96+
.setStatus(Status.DONE)
9597
.setStatusMessage("statusMessage-958704715")
9698
.setTargetId(-815576439)
9799
.setTargetLink("targetLink486368555")
@@ -151,7 +153,7 @@ public void waitTest() throws Exception {
151153
.setEndTime("endTime-1607243192")
152154
.setError(Error.newBuilder().build())
153155
.setHttpErrorMessage("httpErrorMessage1577303431")
154-
.setHttpErrorStatusCode(1386087020)
156+
.setHttpErrorStatusCode(0)
155157
.setId(3355)
156158
.setInsertTime("insertTime966165798")
157159
.setKind("kind3292052")
@@ -161,6 +163,7 @@ public void waitTest() throws Exception {
161163
.setRegion("region-934795532")
162164
.setSelfLink("selfLink1191800166")
163165
.setStartTime("startTime-2129294769")
166+
.setStatus(Status.DONE)
164167
.setStatusMessage("statusMessage-958704715")
165168
.setTargetId(-815576439)
166169
.setTargetLink("targetLink486368555")

0 commit comments

Comments
 (0)