From 1c4bd496f962dd09a98a2ba7860fa9d49bc71d21 Mon Sep 17 00:00:00 2001
From: Xiaozhen Liu <xiaozhenliu@google.com>
Date: Tue, 5 May 2020 09:24:38 -0700
Subject: [PATCH] fix: remove common protos in generated proto list (#455)

* fix proto list logic

* add unit test for apis in different dirs

* update asset baseline

Co-authored-by: Alexander Fenster <fenster@google.com>
---
 .../v1/asset_service_proto_list.json.baseline | 16 +---------
 typescript/src/schema/proto.ts                | 23 ++++++++++++---
 typescript/test/unit/api.ts                   | 29 ++++++++++++++++++-
 3 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/baselines/asset/src/v1/asset_service_proto_list.json.baseline b/baselines/asset/src/v1/asset_service_proto_list.json.baseline
index b5bf323ce..44d0dc015 100644
--- a/baselines/asset/src/v1/asset_service_proto_list.json.baseline
+++ b/baselines/asset/src/v1/asset_service_proto_list.json.baseline
@@ -1,23 +1,9 @@
 [
-  "../../protos/google/api/annotations.proto",
-  "../../protos/google/api/client.proto",
-  "../../protos/google/api/field_behavior.proto",
-  "../../protos/google/api/http.proto",
-  "../../protos/google/api/resource.proto",
   "../../protos/google/cloud/asset/v1/asset_service.proto",
   "../../protos/google/cloud/asset/v1/assets.proto",
   "../../protos/google/cloud/orgpolicy/v1/orgpolicy.proto",
   "../../protos/google/identity/accesscontextmanager/type/device_resources.proto",
   "../../protos/google/identity/accesscontextmanager/v1/access_level.proto",
   "../../protos/google/identity/accesscontextmanager/v1/access_policy.proto",
-  "../../protos/google/identity/accesscontextmanager/v1/service_perimeter.proto",
-  "../../protos/google/protobuf/any.proto",
-  "../../protos/google/protobuf/descriptor.proto",
-  "../../protos/google/protobuf/duration.proto",
-  "../../protos/google/protobuf/empty.proto",
-  "../../protos/google/protobuf/field_mask.proto",
-  "../../protos/google/protobuf/struct.proto",
-  "../../protos/google/protobuf/timestamp.proto",
-  "../../protos/google/rpc/status.proto",
-  "../../protos/google/type/expr.proto"
+  "../../protos/google/identity/accesscontextmanager/v1/service_perimeter.proto"
 ]
diff --git a/typescript/src/schema/proto.ts b/typescript/src/schema/proto.ts
index 8cf695b91..3286b260a 100644
--- a/typescript/src/schema/proto.ts
+++ b/typescript/src/schema/proto.ts
@@ -25,6 +25,13 @@ import {
 import {BundleConfig} from 'src/bundle';
 import {Options} from './naming';
 
+const COMMON_PROTO_LIST = [
+  'google.api',
+  'google.rpc',
+  'google.protobuf',
+  'google.type',
+];
+
 interface MethodDescriptorProto
   extends plugin.google.protobuf.IMethodDescriptorProto {
   longRunning?: plugin.google.longrunning.IOperationInfo;
@@ -617,7 +624,7 @@ export class Proto {
   services: ServicesMap = {};
   allMessages: MessagesMap = {};
   localMessages: MessagesMap = {};
-  fileToGenerate: boolean;
+  fileToGenerate = true;
   // TODO: need to store metadata? address?
 
   // allResourceDatabase: resources that defined by `google.api.resource`
@@ -634,9 +641,17 @@ export class Proto {
         map[`.${parameters.fd.package!}.${message.name!}`] = message;
         return map;
       }, {} as MessagesMap);
-    this.fileToGenerate = parameters.fd.package
-      ? parameters.fd.package.startsWith(parameters.packageName)
-      : false;
+    const protopackage = parameters.fd.package;
+    if (!protopackage || !protopackage.startsWith(parameters.packageName)) {
+      this.fileToGenerate = false;
+    }
+    if (this.fileToGenerate) {
+      for (const commonProto of COMMON_PROTO_LIST) {
+        if (protopackage?.startsWith(commonProto)) {
+          this.fileToGenerate = false;
+        }
+      }
+    }
     this.services = parameters.fd.service
       .filter(service => service.name)
       .map(service =>
diff --git a/typescript/test/unit/api.ts b/typescript/test/unit/api.ts
index d1c4c4dd2..133c2f057 100644
--- a/typescript/test/unit/api.ts
+++ b/typescript/test/unit/api.ts
@@ -66,7 +66,7 @@ describe('src/schema/api.ts', () => {
     const fd3 = new plugin.google.protobuf.FileDescriptorProto();
     fd3.name = 'google/iam/v1/iam_policy.proto';
     fd3.package = 'google.iam.v1';
-    fd2.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
+    fd3.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
     const api = new API([fd1, fd2, fd3], 'google.cloud.test.v1', {
       grpcServiceConfig: new plugin.grpc.service_config.ServiceConfig(),
     });
@@ -75,6 +75,33 @@ describe('src/schema/api.ts', () => {
     ]);
   });
 
+  it('should not return common protos in the proto list', () => {
+    const fd1 = new plugin.google.protobuf.FileDescriptorProto();
+    fd1.name = 'google/cloud/test/v1/test.proto';
+    fd1.package = 'google.cloud.test.v1';
+    fd1.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
+    fd1.service[0].name = 'ZService';
+    fd1.service[0].options = {
+      '.google.api.defaultHost': 'hostname.example.com:443',
+    };
+    const fd2 = new plugin.google.protobuf.FileDescriptorProto();
+    fd2.name = 'google/api/annotations.proto';
+    fd2.package = 'google.api';
+    const fd3 = new plugin.google.protobuf.FileDescriptorProto();
+    fd3.name = 'google/orgpolicy/v1/orgpolicy.proto';
+    fd3.package = 'google.orgpolicy.v1';
+    const fd4 = new plugin.google.protobuf.FileDescriptorProto();
+    fd4.name = 'google/cloud/common_resources.proto';
+    fd4.package = 'google.cloud';
+    const api = new API([fd1, fd2, fd3, fd4], 'google', {
+      grpcServiceConfig: new plugin.grpc.service_config.ServiceConfig(),
+    });
+    assert.deepStrictEqual(api.filesToGenerate, [
+      'google/cloud/test/v1/test.proto',
+      'google/orgpolicy/v1/orgpolicy.proto',
+    ]);
+  });
+
   it('should return lexicographically first service name as mainServiceName', () => {
     const fd1 = new plugin.google.protobuf.FileDescriptorProto();
     fd1.name = 'google/cloud/test/v1/test.proto';