New errors for name / id checks in family constructor (#202)

* New errors for name / id checks in family constructor

* Fixed linting issue

* changes as per review comments

* changes as per review comments

Author: muraliQlogic

handwritten/bigtable/src/family.js

@@ -48,34 +48,22 @@ class Family {
     this.bigtable = table.bigtable;
     this.table = table;
 
-    const name = Family.formatName_(table.name, id);
-
-    this.id = name.split('/').pop();
-    this.name = name;
-  }
-
-  /**
-   * Format the Column Family name into the expected proto format.
-   *
-   * @private
-   *
-   * @param {string} tableName The full formatted table name.
-   * @param {string} id The column family unique-identifier.
-   * @returns {string}
-   *
-   * @example
-   * Family.formatName_(
-   *   'projects/p/zones/z/clusters/c/tables/t',
-   *   'my-family'
-   * );
-   * // 'projects/p/zones/z/clusters/c/tables/t/columnFamilies/my-family'
-   */
-  static formatName_(tableName, id) {
+    var name;
     if (id.includes('/')) {
-      return id;
+      if (id.startsWith(`${table.name}/columnFamilies/`)) {
+        name = id;
+      } else {
+        throw new Error(
+          `Family id '${id}' is not formatted correctly.  
+Please use the format 'follows' or '${table.name}/columnFamilies/my-family'.`
+        );
+      }
+    } else {
+      name = `${table.name}/columnFamilies/${id}`;
     }
 
-    return `${tableName}/columnFamilies/${id}`;
+    this.name = name;
+    this.id = name.split('/').pop();
   }
 
   /**

handwritten/bigtable/src/table.js

@@ -278,7 +278,7 @@ Please use the format 'prezzy' or '${instance.name}/tables/prezzy'.`
           return;
         }
 
-        const family = this.family(resp.name);
+        const family = this.family(id);
         family.metadata = resp;
 
         callback(null, family, resp);

handwritten/bigtable/test/family.js

@@ -85,19 +85,18 @@ describe('Bigtable/Family', function() {
       var family = new Family(TABLE, FAMILY_ID);
       assert.strictEqual(family.name, FAMILY_NAME);
     });
-  });
-
-  describe('formatName_', function() {
-    it('should format the column family name', function() {
-      var formatted = Family.formatName_(TABLE.name, FAMILY_ID);
 
-      assert.strictEqual(formatted, FAMILY_NAME);
+    it('should leave full family names unaltered and localize the id from the name', function() {
+      var family = new Family(TABLE, FAMILY_NAME);
+      assert.strictEqual(family.name, FAMILY_NAME);
+      assert.strictEqual(family.id, FAMILY_ID);
     });
 
-    it('should not re-format the name', function() {
-      var formatted = Family.formatName_(TABLE.name, FAMILY_ID);
-
-      assert.strictEqual(formatted, FAMILY_NAME);
+    it('should throw if family id in wrong format', function() {
+      var id = `/project/bad-project/instances/bad-instance/columnFamiles/${FAMILY_ID}`;
+      assert.throws(function() {
+        new Family(TABLE, id);
+      }, Error);
     });
   });
 

handwritten/bigtable/test/table.js

@@ -281,6 +281,7 @@ describe('Bigtable/Table', function() {
 
   describe('createFamily', function() {
     var COLUMN_ID = 'my-column';
+    var FAMILY_ID = 'test-family';
 
     it('should throw if a id is not provided', function() {
       assert.throws(function() {
@@ -372,12 +373,12 @@ describe('Bigtable/Table', function() {
         callback(null, response);
       };
 
-      table.family = function(name) {
-        assert.strictEqual(name, response.name);
+      table.family = function(id) {
+        assert.strictEqual(id, FAMILY_ID);
         return fakeFamily;
       };
 
-      table.createFamily(COLUMN_ID, function(err, family, apiResponse) {
+      table.createFamily(FAMILY_ID, function(err, family, apiResponse) {
         assert.ifError(err);
         assert.strictEqual(family, fakeFamily);
         assert.strictEqual(family.metadata, response);