fix: do not consider common protos and no-service protos when determi…

…ning package name (#79)
googleapis · Oct 28, 2019 · fdc83b1 · fdc83b1
1 parent 908dd14
commit fdc83b1
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 1 deletion.
diff --git a/typescript/src/schema/naming.ts b/typescript/src/schema/naming.ts
@@ -9,7 +9,11 @@ export class Naming {
   protoPackage: string;
 
   constructor(fileDescriptors: plugin.google.protobuf.IFileDescriptorProto[]) {
-    const protoPackages = fileDescriptors.map(fd => fd.package || '');
+    const protoPackages = fileDescriptors
+      .filter(fd => fd.service && fd.service.length > 0)
+      // LRO is an exception: it's a service but we don't generate any code for it
+      .filter(fd => fd.package !== 'google.longrunning')
+      .map(fd => fd.package || '');
     const prefix = commonPrefix(protoPackages);
     // common prefix must either end with `.`, or be equal to at least one of
     // the packages' prefix

diff --git a/typescript/test/unit/naming.ts b/typescript/test/unit/naming.ts
@@ -22,7 +22,38 @@ describe('schema/naming.ts', () => {
     const descriptor1 = new plugin.google.protobuf.FileDescriptorProto();
     const descriptor2 = new plugin.google.protobuf.FileDescriptorProto();
     descriptor1.package = 'google.namespace.service.v1beta1';
+    descriptor1.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
     descriptor2.package = 'google.namespace.service.v1beta1';
+    descriptor2.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
+    const naming = new Naming([descriptor1, descriptor2]);
+    assert.strictEqual(naming.name, 'Service');
+    assert.strictEqual(naming.productName, 'Service');
+    assert.deepStrictEqual(naming.namespace, ['google', 'namespace']);
+    assert.strictEqual(naming.version, 'v1beta1');
+    assert.strictEqual(naming.protoPackage, 'google.namespace.service.v1beta1');
+  });
+
+  it('ignores files with no services when determining package name', () => {
+    const descriptor1 = new plugin.google.protobuf.FileDescriptorProto();
+    const descriptor2 = new plugin.google.protobuf.FileDescriptorProto();
+    descriptor1.package = 'google.namespace.service.v1beta1';
+    descriptor1.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
+    descriptor2.package = 'google.namespace.service.v1beta2';
+    const naming = new Naming([descriptor1, descriptor2]);
+    assert.strictEqual(naming.name, 'Service');
+    assert.strictEqual(naming.productName, 'Service');
+    assert.deepStrictEqual(naming.namespace, ['google', 'namespace']);
+    assert.strictEqual(naming.version, 'v1beta1');
+    assert.strictEqual(naming.protoPackage, 'google.namespace.service.v1beta1');
+  });
+
+  it('ignores LRO files when determining package name', () => {
+    const descriptor1 = new plugin.google.protobuf.FileDescriptorProto();
+    const descriptor2 = new plugin.google.protobuf.FileDescriptorProto();
+    descriptor1.package = 'google.namespace.service.v1beta1';
+    descriptor1.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
+    descriptor2.package = 'google.longrunning';
+    descriptor2.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
     const naming = new Naming([descriptor1, descriptor2]);
     assert.strictEqual(naming.name, 'Service');
     assert.strictEqual(naming.productName, 'Service');
@@ -34,6 +65,7 @@ describe('schema/naming.ts', () => {
   it('fails on bad package name 1', () => {
     const descriptor = new plugin.google.protobuf.FileDescriptorProto();
     descriptor.package = 'nonamespace';
+    descriptor.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
     assert.throws(() => {
       const naming = new Naming([descriptor]);
     });
@@ -42,6 +74,7 @@ describe('schema/naming.ts', () => {
   it('fails on bad package name 2', () => {
     const descriptor = new plugin.google.protobuf.FileDescriptorProto();
     descriptor.package = '---';
+    descriptor.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
     assert.throws(() => {
       const naming = new Naming([descriptor]);
     });
@@ -50,6 +83,7 @@ describe('schema/naming.ts', () => {
   it('fails on no package name', () => {
     const descriptor = new plugin.google.protobuf.FileDescriptorProto();
     descriptor.package = '';
+    descriptor.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
     assert.throws(() => {
       const naming = new Naming([descriptor]);
     });
@@ -59,7 +93,9 @@ describe('schema/naming.ts', () => {
     const descriptor1 = new plugin.google.protobuf.FileDescriptorProto();
     const descriptor2 = new plugin.google.protobuf.FileDescriptorProto();
     descriptor1.package = 'namespace1.service.v1beta1';
+    descriptor1.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
     descriptor2.package = 'namespace2.service.v1beta1';
+    descriptor2.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
     assert.throws(() => {
       const naming = new Naming([descriptor1, descriptor2]);
     });
@@ -69,7 +105,9 @@ describe('schema/naming.ts', () => {
     const descriptor1 = new plugin.google.protobuf.FileDescriptorProto();
     const descriptor2 = new plugin.google.protobuf.FileDescriptorProto();
     descriptor1.package = 'namespace.service.v1beta1';
+    descriptor1.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
     descriptor2.package = 'namespace.service.v1beta2';
+    descriptor2.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
     assert.throws(() => {
       const naming = new Naming([descriptor1, descriptor2]);
     });
@@ -79,7 +117,9 @@ describe('schema/naming.ts', () => {
     const descriptor1 = new plugin.google.protobuf.FileDescriptorProto();
     const descriptor2 = new plugin.google.protobuf.FileDescriptorProto();
     descriptor1.package = 'namespace.service.v1beta1';
+    descriptor1.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
     descriptor2.package = 'namespace.service';
+    descriptor2.service = [new plugin.google.protobuf.ServiceDescriptorProto()];
     assert.throws(() => {
       const naming = new Naming([descriptor1, descriptor2]);
     });