Skip to content

Commit 2bed7cf

Browse files
vam-googlevam-google
authored
fix: More REST transport fixes (#1003)
1) Generate an empty unit test method even if the method is not supported in a particular transport. Bazel fails test execution, if the test class has no `@Test` methods. In case if there is a service with only unsupported methods, and we do not generate unit tests for those, we would get an empty unit test, which will fail execution in our automatic bazel build pipeline. 2) For fields of `google.protobuf.Value` type set an actual value for it (to avoid non-set vs null value differences after json serialization/deserializaiton roudtrip). This looks like an issue in protobuf support of `google.protobuf.Value` type in JSON and not specific to GAPIC.
1 parent 7f186b5 commit 2bed7cf

File tree

9 files changed

+108
-3
lines changed

9 files changed

+108
-3
lines changed

‎BUILD.bazel

+1
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ proto_library(
7979
"@com_google_protobuf//:empty_proto",
8080
"@com_google_protobuf//:field_mask_proto",
8181
"@com_google_protobuf//:timestamp_proto",
82+
"@com_google_protobuf//:struct_proto",
8283
],
8384
)
8485
java_proto_library(

‎src/main/java/com/google/api/generator/engine/ast/TypeNode.java

+3
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ public enum TypeKind {
7070

7171
public static final TypeNode BYTESTRING =
7272
TypeNode.withReference(ConcreteReference.withClazz(ByteString.class));
73+
public static final TypeNode VALUE =
74+
withReference(
75+
VaporReference.builder().setName("Value").setPakkage("com.google.protobuf").build());
7376

7477
private static final Map<TypeNode, TypeNode> BOXED_TYPE_MAP = createBoxedTypeMap();
7578

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

+26
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,7 @@ private List<MethodDefinition> createTestMethods(
230230
List<MethodDefinition> javaMethods = new ArrayList<>();
231231
for (Method method : service.methods()) {
232232
if (!isSupportedMethod(method)) {
233+
javaMethods.add(createUnsupportedTestMethod(method));
233234
continue;
234235
}
235236
Service matchingService = service;
@@ -760,6 +761,31 @@ protected abstract List<Statement> createStreamingRpcExceptionTestStatements(
760761
Map<String, ResourceName> resourceNames,
761762
Map<String, Message> messageTypes);
762763

764+
protected MethodDefinition createUnsupportedTestMethod(Method method) {
765+
String javaMethodName = JavaStyle.toLowerCamelCase(method.name());
766+
String exceptionTestMethodName = String.format("%sUnsupportedMethodTest", javaMethodName);
767+
768+
List<Statement> methodBody =
769+
Collections.singletonList(
770+
CommentStatement.withComment(
771+
LineComment.withComment(
772+
"The "
773+
+ javaMethodName
774+
+ "() method is not supported in "
775+
+ String.join("+", getTransportContext().transportNames())
776+
+ " transport.\n"
777+
+ "This empty test is generated for technical reasons.")));
778+
779+
return MethodDefinition.builder()
780+
.setAnnotations(Arrays.asList(TEST_ANNOTATION))
781+
.setScope(ScopeNode.PUBLIC)
782+
.setReturnType(TypeNode.VOID)
783+
.setName(exceptionTestMethodName)
784+
.setThrowsExceptions(Arrays.asList(TypeNode.withExceptionClazz(Exception.class)))
785+
.setBody(methodBody)
786+
.build();
787+
}
788+
763789
protected List<Statement> createRpcExceptionTestStatements(
764790
Method method,
765791
List<MethodArgument> methodSignature,

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

+14
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,20 @@ public static Expr createValue(
168168
.setStaticReferenceType(field.type())
169169
.setMethodName("newBuilder")
170170
.build();
171+
if (field.type().equals(TypeNode.VALUE)) {
172+
newBuilderExpr =
173+
MethodInvocationExpr.builder()
174+
.setExprReferenceExpr(newBuilderExpr)
175+
.setMethodName("setBoolValue")
176+
.setArguments(
177+
ValueExpr.withValue(
178+
PrimitiveValue.builder()
179+
.setType(TypeNode.BOOLEAN)
180+
.setValue("true")
181+
.build()))
182+
.build();
183+
}
184+
171185
return MethodInvocationExpr.builder()
172186
.setExprReferenceExpr(newBuilderExpr)
173187
.setMethodName("build")

‎src/test/java/com/google/api/generator/gapic/composer/defaultvalue/DefaultValueComposerTest.java

+23
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import com.google.api.generator.testutils.LineFormatter;
3030
import com.google.protobuf.ByteString;
3131
import com.google.protobuf.Descriptors.FileDescriptor;
32+
import com.google.protobuf.StructProto;
3233
import com.google.showcase.v1beta1.EchoOuterClass;
3334
import com.google.testgapic.v1beta1.LockerProto;
3435
import java.util.Arrays;
@@ -410,6 +411,28 @@ public void createSimpleMessage_containsMessagesEnumsAndResourceName() {
410411
writerVisitor.write());
411412
}
412413

414+
@Test
415+
public void createSimpleMessage_valueField() {
416+
FileDescriptor echoFileDescriptor =
417+
com.google.showcase.grpcrest.v1beta1.EchoGrpcrest.getDescriptor();
418+
Map<String, Message> messageTypes = Parser.parseMessages(echoFileDescriptor);
419+
messageTypes.putAll(Parser.parseMessages(StructProto.getDescriptor()));
420+
Map<String, ResourceName> typeStringsToResourceNames =
421+
Parser.parseResourceNames(echoFileDescriptor);
422+
Message message = messageTypes.get("com.google.showcase.grpcrest.v1beta1.EchoResponse");
423+
Expr expr =
424+
DefaultValueComposer.createSimpleMessageBuilderValue(
425+
message, typeStringsToResourceNames, messageTypes, null);
426+
expr.accept(writerVisitor);
427+
assertEquals(
428+
"EchoResponse.newBuilder()"
429+
+ ".setContent(\"content951530617\")"
430+
+ ".setSeverity(Severity.forNumber(0))"
431+
+ ".setValueField(Value.newBuilder().setBoolValue(true).build())"
432+
+ ".build()",
433+
writerVisitor.write());
434+
}
435+
413436
@Test
414437
public void createSimpleMessage_containsRepeatedField() {
415438
FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor();

‎src/test/java/com/google/api/generator/gapic/composer/grpcrest/GrpcRestTestProtoLoader.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import com.google.longrunning.OperationsProto;
3030
import com.google.protobuf.Descriptors.FileDescriptor;
3131
import com.google.protobuf.Descriptors.ServiceDescriptor;
32+
import com.google.protobuf.StructProto;
3233
import com.google.showcase.grpcrest.v1beta1.EchoGrpcrest;
3334
import java.nio.file.Path;
3435
import java.nio.file.Paths;
@@ -57,9 +58,9 @@ public GapicContext parseShowcaseEcho() {
5758
assertEquals("Echo", echoServiceDescriptor.getName());
5859

5960
Map<String, Message> messageTypes = Parser.parseMessages(echoFileDescriptor);
60-
Map<String, Message> operationMessageTypes =
61-
Parser.parseMessages(OperationsProto.getDescriptor());
62-
messageTypes.putAll(operationMessageTypes);
61+
messageTypes.putAll(Parser.parseMessages(OperationsProto.getDescriptor()));
62+
messageTypes.putAll(Parser.parseMessages(StructProto.getDescriptor()));
63+
6364
Map<String, ResourceName> resourceNames = Parser.parseResourceNames(echoFileDescriptor);
6465
Set<ResourceName> outputResourceNames = new HashSet<>();
6566
List<Service> services =

‎src/test/java/com/google/api/generator/gapic/composer/grpcrest/goldens/EchoClientHttpJsonTest.golden

+21
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import com.google.longrunning.Operation;
1818
import com.google.protobuf.Any;
1919
import com.google.protobuf.Duration;
2020
import com.google.protobuf.Timestamp;
21+
import com.google.protobuf.Value;
2122
import com.google.rpc.Status;
2223
import com.google.showcase.grpcrest.v1beta1.stub.HttpJsonEchoStub;
2324
import java.io.IOException;
@@ -72,6 +73,7 @@ public class EchoClientHttpJsonTest {
7273
EchoResponse.newBuilder()
7374
.setContent("content951530617")
7475
.setSeverity(Severity.forNumber(0))
76+
.setValueField(Value.newBuilder().setBoolValue(true).build())
7577
.build();
7678
mockService.addResponse(expectedResponse);
7779

@@ -121,6 +123,7 @@ public class EchoClientHttpJsonTest {
121123
EchoResponse.newBuilder()
122124
.setContent("content951530617")
123125
.setSeverity(Severity.forNumber(0))
126+
.setValueField(Value.newBuilder().setBoolValue(true).build())
124127
.build();
125128
mockService.addResponse(expectedResponse);
126129

@@ -166,6 +169,7 @@ public class EchoClientHttpJsonTest {
166169
EchoResponse.newBuilder()
167170
.setContent("content951530617")
168171
.setSeverity(Severity.forNumber(0))
172+
.setValueField(Value.newBuilder().setBoolValue(true).build())
169173
.build();
170174
mockService.addResponse(expectedResponse);
171175

@@ -211,6 +215,7 @@ public class EchoClientHttpJsonTest {
211215
EchoResponse.newBuilder()
212216
.setContent("content951530617")
213217
.setSeverity(Severity.forNumber(0))
218+
.setValueField(Value.newBuilder().setBoolValue(true).build())
214219
.build();
215220
mockService.addResponse(expectedResponse);
216221

@@ -256,6 +261,7 @@ public class EchoClientHttpJsonTest {
256261
EchoResponse.newBuilder()
257262
.setContent("content951530617")
258263
.setSeverity(Severity.forNumber(0))
264+
.setValueField(Value.newBuilder().setBoolValue(true).build())
259265
.build();
260266
mockService.addResponse(expectedResponse);
261267

@@ -301,6 +307,7 @@ public class EchoClientHttpJsonTest {
301307
EchoResponse.newBuilder()
302308
.setContent("content951530617")
303309
.setSeverity(Severity.forNumber(0))
310+
.setValueField(Value.newBuilder().setBoolValue(true).build())
304311
.build();
305312
mockService.addResponse(expectedResponse);
306313

@@ -346,6 +353,7 @@ public class EchoClientHttpJsonTest {
346353
EchoResponse.newBuilder()
347354
.setContent("content951530617")
348355
.setSeverity(Severity.forNumber(0))
356+
.setValueField(Value.newBuilder().setBoolValue(true).build())
349357
.build();
350358
mockService.addResponse(expectedResponse);
351359

@@ -391,6 +399,7 @@ public class EchoClientHttpJsonTest {
391399
EchoResponse.newBuilder()
392400
.setContent("content951530617")
393401
.setSeverity(Severity.forNumber(0))
402+
.setValueField(Value.newBuilder().setBoolValue(true).build())
394403
.build();
395404
mockService.addResponse(expectedResponse);
396405

@@ -805,4 +814,16 @@ public class EchoClientHttpJsonTest {
805814
// Expected exception.
806815
}
807816
}
817+
818+
@Test
819+
public void chatUnsupportedMethodTest() throws Exception {
820+
// The chat() method is not supported in REST transport.
821+
// This empty test is generated for technical reasons.
822+
}
823+
824+
@Test
825+
public void noBindingUnsupportedMethodTest() throws Exception {
826+
// The noBinding() method is not supported in REST transport.
827+
// This empty test is generated for technical reasons.
828+
}
808829
}

‎src/test/java/com/google/api/generator/gapic/composer/grpcrest/goldens/EchoClientTest.golden

+12
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import com.google.protobuf.AbstractMessage;
2222
import com.google.protobuf.Any;
2323
import com.google.protobuf.Duration;
2424
import com.google.protobuf.Timestamp;
25+
import com.google.protobuf.Value;
2526
import com.google.rpc.Status;
2627
import io.grpc.StatusRuntimeException;
2728
import java.io.IOException;
@@ -82,6 +83,7 @@ public class EchoClientTest {
8283
EchoResponse.newBuilder()
8384
.setContent("content951530617")
8485
.setSeverity(Severity.forNumber(0))
86+
.setValueField(Value.newBuilder().setBoolValue(true).build())
8587
.build();
8688
mockEcho.addResponse(expectedResponse);
8789

@@ -124,6 +126,7 @@ public class EchoClientTest {
124126
EchoResponse.newBuilder()
125127
.setContent("content951530617")
126128
.setSeverity(Severity.forNumber(0))
129+
.setValueField(Value.newBuilder().setBoolValue(true).build())
127130
.build();
128131
mockEcho.addResponse(expectedResponse);
129132

@@ -163,6 +166,7 @@ public class EchoClientTest {
163166
EchoResponse.newBuilder()
164167
.setContent("content951530617")
165168
.setSeverity(Severity.forNumber(0))
169+
.setValueField(Value.newBuilder().setBoolValue(true).build())
166170
.build();
167171
mockEcho.addResponse(expectedResponse);
168172

@@ -202,6 +206,7 @@ public class EchoClientTest {
202206
EchoResponse.newBuilder()
203207
.setContent("content951530617")
204208
.setSeverity(Severity.forNumber(0))
209+
.setValueField(Value.newBuilder().setBoolValue(true).build())
205210
.build();
206211
mockEcho.addResponse(expectedResponse);
207212

@@ -241,6 +246,7 @@ public class EchoClientTest {
241246
EchoResponse.newBuilder()
242247
.setContent("content951530617")
243248
.setSeverity(Severity.forNumber(0))
249+
.setValueField(Value.newBuilder().setBoolValue(true).build())
244250
.build();
245251
mockEcho.addResponse(expectedResponse);
246252

@@ -280,6 +286,7 @@ public class EchoClientTest {
280286
EchoResponse.newBuilder()
281287
.setContent("content951530617")
282288
.setSeverity(Severity.forNumber(0))
289+
.setValueField(Value.newBuilder().setBoolValue(true).build())
283290
.build();
284291
mockEcho.addResponse(expectedResponse);
285292

@@ -319,6 +326,7 @@ public class EchoClientTest {
319326
EchoResponse.newBuilder()
320327
.setContent("content951530617")
321328
.setSeverity(Severity.forNumber(0))
329+
.setValueField(Value.newBuilder().setBoolValue(true).build())
322330
.build();
323331
mockEcho.addResponse(expectedResponse);
324332

@@ -358,6 +366,7 @@ public class EchoClientTest {
358366
EchoResponse.newBuilder()
359367
.setContent("content951530617")
360368
.setSeverity(Severity.forNumber(0))
369+
.setValueField(Value.newBuilder().setBoolValue(true).build())
361370
.build();
362371
mockEcho.addResponse(expectedResponse);
363372

@@ -400,6 +409,7 @@ public class EchoClientTest {
400409
EchoResponse.newBuilder()
401410
.setContent("content951530617")
402411
.setSeverity(Severity.forNumber(0))
412+
.setValueField(Value.newBuilder().setBoolValue(true).build())
403413
.build();
404414
mockEcho.addResponse(expectedResponse);
405415
ExpandRequest request =
@@ -781,6 +791,7 @@ public class EchoClientTest {
781791
EchoResponse.newBuilder()
782792
.setContent("content951530617")
783793
.setSeverity(Severity.forNumber(0))
794+
.setValueField(Value.newBuilder().setBoolValue(true).build())
784795
.build();
785796
mockEcho.addResponse(expectedResponse);
786797
EchoRequest request =
@@ -839,6 +850,7 @@ public class EchoClientTest {
839850
EchoResponse.newBuilder()
840851
.setContent("content951530617")
841852
.setSeverity(Severity.forNumber(0))
853+
.setValueField(Value.newBuilder().setBoolValue(true).build())
842854
.build();
843855
mockEcho.addResponse(expectedResponse);
844856

‎src/test/proto/echo_grpcrest.proto

+4
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import "google/api/field_behavior.proto";
2020
import "google/api/resource.proto";
2121
import "google/longrunning/operations.proto";
2222
import "google/protobuf/duration.proto";
23+
import "google/protobuf/struct.proto";
2324
import "google/protobuf/timestamp.proto";
2425
import "google/rpc/status.proto";
2526

@@ -210,6 +211,9 @@ message EchoResponse {
210211

211212
// The severity specified in the request.
212213
Severity severity = 2;
214+
215+
// Value field to test special case in Value type serialization.
216+
google.protobuf.Value value_field = 3;
213217
}
214218

215219
// Tests name collisions with java.lang.Object.

0 commit comments

Comments
 (0)