feat: fail for misconfigured service and longrunning method (#456)

* throw error for misconfigured LR method * test set up * add more checks and unit test for defaulthost and LRO * remove packages * clean up * remove extra line * throw warning if the service has empty defaulthost option * feedback
googleapis · May 5, 2020 · cfaef54 · cfaef54
1 parent 4cbf731
commit cfaef54
Show file tree

Hide file tree

Showing 4 changed files with 121 additions and 3 deletions.
diff --git a/typescript/src/schema/api.ts b/typescript/src/schema/api.ts
@@ -99,7 +99,20 @@ export class API {
         servicesList.push(...fd.service!);
         return servicesList;
       }, [] as plugin.google.protobuf.IServiceDescriptorProto[])
-      .filter(service => service?.options?.['.google.api.defaultHost'])
+      .filter(service => {
+        if (!service.options || !service.options['.google.api.defaultHost']) {
+          throw new Error(
+            `service ${service.name} is missing option google.api.default_host`
+          );
+        }
+        const defaultHost = service!.options!['.google.api.defaultHost']!;
+        if (defaultHost.length === 0) {
+          console.warn(
+            `service ${service.name} google.api.default_host is empty`
+          );
+        }
+        return service?.options?.['.google.api.defaultHost'];
+      })
       .sort((service1, service2) =>
         service1.name!.localeCompare(service2.name!)
       )

diff --git a/typescript/src/schema/proto.ts b/typescript/src/schema/proto.ts
@@ -90,8 +90,17 @@ export interface MessagesMap {
 // methods of the given service, to use in templates.
 
 function longrunning(method: MethodDescriptorProto) {
-  if (method.options?.['.google.longrunning.operationInfo']) {
-    return method.options['.google.longrunning.operationInfo']!;
+  if (
+    method.outputType &&
+    method.outputType === '.google.longrunning.Operation'
+  ) {
+    if (!method.options?.['.google.longrunning.operationInfo']) {
+      throw Error(
+        `rpc ${method.name} returns google.longrunning.Operation but is missing option google.longrunning.operation_info`
+      );
+    } else {
+      return method.options!['.google.longrunning.operationInfo']!;
+    }
   }
   return undefined;
 }
@@ -333,6 +342,17 @@ function augmentMethod(
     },
     method
   ) as MethodDescriptorProto;
+  if (method.longRunning) {
+    if (!method.longRunningMetadataType) {
+      throw Error(
+        `rpc ${method.name} has google.longrunning.operation_info but is missing option google.longrunning.operation_info.metadata_type`
+      );
+    } else if (!method.longRunningResponseType) {
+      throw Error(
+        `rpc ${method.name} has google.longrunning.operation_info but is missing option google.longrunning.operation_info.response_type`
+      );
+    }
+  }
   const bundleConfigs = parameters.service.bundleConfigs;
   if (bundleConfigs) {
     for (const bc of bundleConfigs) {

diff --git a/typescript/test/unit/api.ts b/typescript/test/unit/api.ts
@@ -37,6 +37,19 @@ describe('src/schema/api.ts', () => {
     ]);
   });
 
+  it('throw error if an api does not have default host', () => {
+    const fd = new plugin.google.protobuf.FileDescriptorProto();
+    fd.name = 'google/cloud/test/v1/test.proto';
+    fd.package = 'google.cloud.test.v1';
+    fd.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
+    fd.service[0].name = 'ZService';
+    assert.throws(() => {
+      new API([fd], 'google.cloud.test.v1', {
+        grpcServiceConfig: new plugin.grpc.service_config.ServiceConfig(),
+      });
+    }, new Error('service ZService is missing option google.api.default_host'));
+  });
+
   it('should not return common protos in the list of protos', () => {
     const fd1 = new plugin.google.protobuf.FileDescriptorProto();
     fd1.name = 'google/cloud/test/v1/test.proto';

diff --git a/typescript/test/unit/proto.ts b/typescript/test/unit/proto.ts
@@ -179,4 +179,76 @@ describe('src/schema/proto.ts', () => {
       );
     });
   });
+  describe('throw error for misconfigured LRO', () => {
+    it('throw error if method returns Operation, but without operation_info option', () => {
+      const fd = new plugin.google.protobuf.FileDescriptorProto();
+      fd.name = 'google/cloud/showcase/v1beta1/test.proto';
+      fd.package = 'google.cloud.showcase.v1beta1';
+      fd.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
+      fd.service[0].name = 'service';
+      fd.service[0].method = [
+        new plugin.google.protobuf.MethodDescriptorProto(),
+      ];
+      fd.service[0].method[0] = new plugin.google.protobuf.MethodDescriptorProto();
+      fd.service[0].method[0].name = 'Test';
+      fd.service[0].method[0].outputType = '.google.longrunning.Operation';
+      const options: Options = {
+        grpcServiceConfig: new plugin.grpc.service_config.ServiceConfig(),
+      };
+      const allMessages: MessagesMap = {};
+      fd.messageType
+        .filter(message => message.name)
+        .forEach(message => {
+          allMessages['.' + fd.package! + '.' + message.name!] = message;
+        });
+      const commentsMap = new CommentsMap([fd]);
+      assert.throws(() => {
+        new Proto({
+          fd,
+          packageName: 'google.cloud.showcase.v1beta1',
+          allMessages,
+          allResourceDatabase: new ResourceDatabase(),
+          resourceDatabase: new ResourceDatabase(),
+          options,
+          commentsMap,
+        });
+      }, new Error('rpc Test returns google.longrunning.Operation but is missing option google.longrunning.operation_info'));
+    });
+    it('throw error if method returns Operation, but without operation_info option', () => {
+      const fd = new plugin.google.protobuf.FileDescriptorProto();
+      fd.name = 'google/cloud/showcase/v1beta1/test.proto';
+      fd.package = 'google.cloud.showcase.v1beta1';
+      fd.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
+      fd.service[0].name = 'service';
+      fd.service[0].method = [
+        new plugin.google.protobuf.MethodDescriptorProto(),
+      ];
+      fd.service[0].method[0] = new plugin.google.protobuf.MethodDescriptorProto();
+      fd.service[0].method[0].name = 'Test';
+      fd.service[0].method[0].outputType = '.google.longrunning.Operation';
+      const options: Options = {
+        grpcServiceConfig: new plugin.grpc.service_config.ServiceConfig(),
+      };
+      fd.service[0].method[0].options = new plugin.google.protobuf.MethodOptions();
+      fd.service[0].method[0].options['.google.longrunning.operationInfo'] = {};
+      const allMessages: MessagesMap = {};
+      fd.messageType
+        .filter(message => message.name)
+        .forEach(message => {
+          allMessages['.' + fd.package! + '.' + message.name!] = message;
+        });
+      const commentsMap = new CommentsMap([fd]);
+      assert.throws(() => {
+        new Proto({
+          fd,
+          packageName: 'google.cloud.showcase.v1beta1',
+          allMessages,
+          allResourceDatabase: new ResourceDatabase(),
+          resourceDatabase: new ResourceDatabase(),
+          options,
+          commentsMap,
+        });
+      }, new Error('rpc Test has google.longrunning.operation_info but is missing option google.longrunning.operation_info.metadata_type'));
+    });
+  });
 });