From 0ea26b076c02617c1527e13d848272f410f780f2 Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Sun, 7 Aug 2016 16:21:53 -0400 Subject: [PATCH] bigtable: throw when required information is missing (#1452) --- package.json | 3 ++- packages/bigtable/src/index.js | 17 +++++++++++++++++ packages/bigtable/src/row.js | 15 +++++++++++++-- packages/bigtable/src/table.js | 18 ++++++++++++++++++ packages/bigtable/test/filter.js | 4 +--- packages/bigtable/test/index.js | 23 ++++++++++++++++++++++- packages/bigtable/test/row.js | 6 ++++++ packages/bigtable/test/table.js | 24 +++++++++++++++++++++--- 8 files changed, 100 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index 22d3c421143..79b0e2fa44b 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,8 @@ "lint": "jshint scripts/ packages/ system-test/ test/ && jscs packages/ system-test/ test/", "test": "npm run docs && mocha test/docs.js packages/*/test/*.js", "system-test": "mocha packages/*/system-test/*.js --no-timeouts --bail", - "coveralls": "istanbul cover _mocha --report lcovonly -- --no-timeouts --bail packages/*/test/*.js -R spec && cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js && rm -rf ./coverage" + "cover": "istanbul cover _mocha --report lcovonly -- --no-timeouts --bail packages/*/test/*.js -R spec", + "coveralls": "npm run cover && cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js && rm -rf ./coverage" }, "license": "Apache-2.0", "engines": { diff --git a/packages/bigtable/src/index.js b/packages/bigtable/src/index.js index 195bd34362e..457f14847de 100644 --- a/packages/bigtable/src/index.js +++ b/packages/bigtable/src/index.js @@ -56,6 +56,9 @@ var PKG = require('../package.json'); * * @resource [Creating a Cloud Bigtable Cluster]{@link https://cloud.google.com/bigtable/docs/creating-compute-instance} * + * @throws {error} If a cluster is not provided. + * @throws {error} If a zone is not provided. + * * @param {object=} options - [Configuration object](#/docs). * @param {string} options.cluster - The cluster name that hosts your tables. * @param {string|module:compute/zone} options.zone - The zone in which your @@ -278,6 +281,14 @@ function Bigtable(options) { return new Bigtable(options); } + if (!options.cluster) { + throw new Error('A cluster must be provided to interact with Bigtable.'); + } + + if (!options.zone) { + throw new Error('A zone must be provided to interact with Bigtable.'); + } + options = extend({}, options, { zone: options.zone.name || options.zone }); @@ -337,6 +348,8 @@ Bigtable.formatTableName_ = function(name) { * @resource [Designing Your Schema]{@link https://cloud.google.com/bigtable/docs/schema-design} * @resource [Splitting Keys]{@link https://cloud.google.com/bigtable/docs/managing-tables#splits} * + * @throws {error} If a name is not provided. + * * @param {string} name - The name of the table. * @param {object=} options - Table creation options. * @param {object|string[]} options.families - Column families to be created @@ -409,6 +422,10 @@ Bigtable.prototype.createTable = function(name, options, callback) { options = {}; } + if (!name) { + throw new Error('A name is required to create a table.'); + } + var protoOpts = { service: 'BigtableTableService', method: 'createTable' diff --git a/packages/bigtable/src/row.js b/packages/bigtable/src/row.js index 4f920994f19..df26f92ac91 100644 --- a/packages/bigtable/src/row.js +++ b/packages/bigtable/src/row.js @@ -48,6 +48,11 @@ var RowError = createErrorClass('RowError', function(row) { this.message = 'Unknown row: ' + row + '.'; }); +/*! Developer Documentation + * + * @param {module:bigtable/table} table - The row's parent Table instance. + * @param {string} key - The key for this row. + */ /** * Create a Row object to interact with your table rows. * @@ -67,7 +72,7 @@ var RowError = createErrorClass('RowError', function(row) { * var table = bigtable.table('prezzy'); * var row = table.row('gwashington'); */ -function Row(table, name) { +function Row(table, key) { var methods = { /** @@ -87,7 +92,7 @@ function Row(table, name) { var config = { parent: table, methods: methods, - id: name + id: key }; common.GrpcServiceObject.call(this, config); @@ -297,6 +302,8 @@ Row.prototype.create = function(entry, callback) { * transformed into writes. Rules are applied in order, meaning that earlier * rules will affect the results of later ones. * + * @throws {error} If no rules are provided. + * * @param {object|object[]} rules - The rules to apply to this row. * @param {function} callback - The callback function. * @param {?error} callback.err - An error returned while making this @@ -334,6 +341,10 @@ Row.prototype.create = function(entry, callback) { * ], callback); */ Row.prototype.createRules = function(rules, callback) { + if (!rules || rules.length === 0) { + throw new Error('At least one rule must be provided.'); + } + rules = arrify(rules).map(function(rule) { var column = Mutation.parseColumnName(rule.column); var ruleData = { diff --git a/packages/bigtable/src/table.js b/packages/bigtable/src/table.js index 81bfe35d879..71664bc2210 100644 --- a/packages/bigtable/src/table.js +++ b/packages/bigtable/src/table.js @@ -250,6 +250,8 @@ Table.formatRowRange_ = function(range) { * * @resource [Garbage Collection Proto Docs]{@link https://github.com/googleapis/googleapis/blob/master/google/bigtable/admin/table/v1/bigtable_table_data.proto#L59} * + * @throws {error} If a name is not provided. + * * @param {string} name - The name of column family. * @param {string|object=} rule - Garbage collection rule. * @param {object=} rule.age - Delete cells in a column older than the given @@ -294,6 +296,10 @@ Table.prototype.createFamily = function(name, rule, callback) { rule = null; } + if (!name) { + throw new Error('A name is required to create a family.'); + } + var grpcOpts = { service: 'BigtableTableService', method: 'createColumnFamily' @@ -383,6 +389,8 @@ Table.prototype.deleteRows = function(options, callback) { /** * Get a reference to a Table Family. * + * @throws {error} If a name is not provided. + * * @param {string} name - The family name. * @return {module:bigtable/family} * @@ -390,6 +398,10 @@ Table.prototype.deleteRows = function(options, callback) { * var family = table.family('my-family'); */ Table.prototype.family = function(name) { + if (!name) { + throw new Error('A family name must be provided.'); + } + return new Family(this, name); }; @@ -769,6 +781,8 @@ Table.prototype.mutate = function(entries, callback) { /** * Get a reference to a table row. * + * @throws {error} If a key is not provided. + * * @param {string} key - The row key. * @return {module:bigtable/row} * @@ -776,6 +790,10 @@ Table.prototype.mutate = function(entries, callback) { * var row = table.row('lincoln'); */ Table.prototype.row = function(key) { + if (!key) { + throw new Error('A row key must be provided.'); + } + return new Row(this, key); }; diff --git a/packages/bigtable/test/filter.js b/packages/bigtable/test/filter.js index 50fa3e058d3..717b840ae9a 100644 --- a/packages/bigtable/test/filter.js +++ b/packages/bigtable/test/filter.js @@ -80,11 +80,9 @@ describe('Bigtable/Filter', function() { }); it('should throw an error for unknown types', function() { - var errorMessage = /Can\'t convert to RegExp String from unknown type\./; - assert.throws(function() { Filter.convertToRegExpString(true); - }, errorMessage); + }, /Can\'t convert to RegExp String from unknown type\./); }); }); diff --git a/packages/bigtable/test/index.js b/packages/bigtable/test/index.js index 441870f47e6..82af59c9d85 100644 --- a/packages/bigtable/test/index.js +++ b/packages/bigtable/test/index.js @@ -106,12 +106,27 @@ describe('Bigtable', function() { fakeUtil.normalizeArguments = normalizeArguments; }); + it('should throw if a cluster is not provided', function() { + assert.throws(function() { + new Bigtable({}); + }, /A cluster must be provided to interact with Bigtable\./); + }); + + it('should throw if a zone is not provided', function() { + assert.throws(function() { + new Bigtable({ + cluster: CLUSTER + }); + }, /A zone must be provided to interact with Bigtable\./); + }); + it('should leave the original options unaltered', function() { var fakeOptions = { a: 'a', b: 'b', c: 'c', - zone: 'zone' + cluster: CLUSTER, + zone: ZONE }; var bigtable = new Bigtable(fakeOptions); @@ -172,6 +187,12 @@ describe('Bigtable', function() { describe('createTable', function() { var TABLE_ID = 'my-table'; + it('should throw if a name is not provided', function() { + assert.throws(function() { + bigtable.createTable(); + }, /A name is required to create a table\./); + }); + it('should provide the proper request options', function(done) { bigtable.request = function(protoOpts, reqOpts) { assert.deepEqual(protoOpts, { diff --git a/packages/bigtable/test/row.js b/packages/bigtable/test/row.js index 6c2184ff5dc..49c507be125 100644 --- a/packages/bigtable/test/row.js +++ b/packages/bigtable/test/row.js @@ -258,6 +258,12 @@ describe('Bigtable/Row', function() { increment: 1 }]; + it('should throw if a rule is not provided', function() { + assert.throws(function() { + row.createRules(); + }, /At least one rule must be provided\./); + }); + it('should read/modify/write rules', function(done) { row.request = function(grpcOpts, reqOpts, callback) { assert.deepEqual(grpcOpts, { diff --git a/packages/bigtable/test/table.js b/packages/bigtable/test/table.js index 69f1eaa78a5..27986f2424b 100644 --- a/packages/bigtable/test/table.js +++ b/packages/bigtable/test/table.js @@ -217,6 +217,12 @@ describe('Bigtable/Table', function() { describe('createFamily', function() { var COLUMN_ID = 'my-column'; + it('should throw if a name is not provided', function() { + assert.throws(function() { + table.createFamily(); + }, /A name is required to create a family\./); + }); + it('should provide the proper request options', function(done) { table.request = function(grpcOpts, reqOpts) { assert.deepEqual(grpcOpts, { @@ -357,6 +363,12 @@ describe('Bigtable/Table', function() { describe('family', function() { var FAMILY_ID = 'test-family'; + it('should throw if a name is not provided', function() { + assert.throws(function() { + table.family(); + }, /A family name must be provided\./); + }); + it('should create a family with the proper arguments', function() { var family = table.family(FAMILY_ID); @@ -806,14 +818,20 @@ describe('Bigtable/Table', function() { }); describe('row', function() { - var ROW_ID = 'test-row'; + var KEY = 'test-row'; + + it('should throw if a key is not provided', function() { + assert.throws(function() { + table.row(); + }, /A row key must be provided\./); + }); it('should return a Row object', function() { - var row = table.row(ROW_ID); + var row = table.row(KEY); assert(row instanceof FakeRow); assert.strictEqual(row.calledWith_[0], table); - assert.strictEqual(row.calledWith_[1], ROW_ID); + assert.strictEqual(row.calledWith_[1], KEY); }); });