From 6ee22f5afd87dc13eef9156d175bd17ba5cd9e83 Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Mon, 12 Mar 2018 16:07:42 -0400 Subject: [PATCH] remove non-paginated methods --- src/index.js | 44 --------------------- src/instance.js | 41 ++----------------- system-test/bigtable.js | 34 +--------------- test/index.js | 88 +++++------------------------------------ test/instance.js | 81 +++++++++++++++---------------------- 5 files changed, 45 insertions(+), 243 deletions(-) diff --git a/src/index.js b/src/index.js index 898102820..555777c3b 100644 --- a/src/index.js +++ b/src/index.js @@ -570,10 +570,6 @@ Bigtable.prototype.getInstances = function(gaxOptions, callback) { parent: this.projectName, }; - // @TODO this option shouldn't exist in the GAPIC client. - // Ref: https://github.com/googleapis/nodejs-bigtable/pull/35/files#r173892576 - // gaxOptions.autoPaginate = false; - this.request( { client: 'BigtableInstanceAdminClient', @@ -598,40 +594,6 @@ Bigtable.prototype.getInstances = function(gaxOptions, callback) { ); }; -/** - * Get {@link Instance} objects for all of your Compute instances as a readable - * object stream. - * - * @param {object} [query] Configuration object. See - * {@link Bigtable#getInstances} for a complete list of options. - * @returns {stream} - * - * @example - * const Bigtable = require('@google-cloud/bigtable'); - * const bigtable = new Bigtable(); - * - * bigtable.getInstancesStream() - * .on('error', console.error) - * .on('data', function(instance) { - * // `instance` is an Instance object. - * }) - * .on('end', function() { - * // All instances retrieved. - * }); - * - * //- - * // If you anticipate many results, you can end a stream early to prevent - * // unnecessary processing and API requests. - * //- - * bigtable.getInstancesStream() - * .on('data', function(instance) { - * this.end(); - * }); - */ -Bigtable.prototype.getInstancesStream = common.paginator.streamify( - 'getInstances' -); - /** * Get a reference to a Compute instance. * @@ -745,12 +707,6 @@ Bigtable.prototype.request = function(config, callback) { } }; -/*! Developer Documentation - * - * These methods can be auto-paginated. - */ -common.paginator.extend(Bigtable, ['getInstances']); - /*! Developer Documentation * * All async methods (except for streams) will return a Promise in the event diff --git a/src/instance.js b/src/instance.js index 4332fe60f..2a82e01ab 100644 --- a/src/instance.js +++ b/src/instance.js @@ -533,13 +533,9 @@ Instance.prototype.getClusters = function(gaxOptions, callback) { } var reqOpts = { - parent: this.projectName, + parent: this.id, }; - // @TODO this option shouldn't exist in the GAPIC client. - // Ref: https://github.com/googleapis/nodejs-bigtable/pull/35/files#r173892576 - gaxOptions.autoPaginate = false; - this.bigtable.request( { client: 'BigtableInstanceAdminClient', @@ -553,7 +549,7 @@ Instance.prototype.getClusters = function(gaxOptions, callback) { return; } - var clusters = resp.clusters.map(function(instanceData) { + var clusters = resp.clusters.map(function(clusterObj) { var cluster = self.cluster(clusterObj.name); cluster.metadata = clusterObj; return cluster; @@ -564,37 +560,6 @@ Instance.prototype.getClusters = function(gaxOptions, callback) { ); }; -/** - * Get {@link Cluster} objects for all of your clusters as a readable object - * stream. - * - * @param {object} [query] Configuration object. See - * {@link Instance#getClusters} for a complete list of options. - * @returns {stream} - * - * @example - * instance.getClustersStream() - * .on('error', console.error) - * .on('data', function(cluster) { - * // `cluster` is a Cluster object. - * }) - * .on('end', function() { - * // All clusters retrieved. - * }); - * - * //- - * // If you anticipate many results, you can end a stream early to prevent - * // unnecessary processing and API requests. - * //- - * instance.getClustersStream() - * .on('data', function(cluster) { - * this.end(); - * }); - */ -Instance.prototype.getClustersStream = common.paginator.streamify( - 'getClusters' -); - /** * Get the instance metadata. * @@ -846,7 +811,7 @@ Instance.prototype.table = function(name) { * * These methods can be auto-paginated. */ -common.paginator.extend(Instance, ['getClusters', 'getTables']); +common.paginator.extend(Instance, ['getTables']); /*! Developer Documentation * diff --git a/system-test/bigtable.js b/system-test/bigtable.js index e4b5a73fe..4b8cafa09 100644 --- a/system-test/bigtable.js +++ b/system-test/bigtable.js @@ -21,7 +21,6 @@ var async = require('async'); var uuid = require('uuid'); var Bigtable = require('../'); -var Instance = require('../src/instance.js'); var Cluster = require('../src/cluster.js'); var Table = require('../src/table.js'); var Family = require('../src/family.js'); @@ -87,7 +86,7 @@ describe('Bigtable', function() { }); }); - describe.only('instances', function() { + describe('instances', function() { it('should get a list of instances', function(done) { bigtable.getInstances(function(err, instances) { assert.ifError(err); @@ -96,22 +95,6 @@ describe('Bigtable', function() { }); }); - it('should get a list of instances in stream mode', function(done) { - var instances = []; - - bigtable - .getInstancesStream() - .on('error', done) - .on('data', function(instance) { - assert(instance instanceof Instance); - instances.push(instance); - }) - .on('end', function() { - assert(instances.length > 0); - done(); - }); - }); - it('should check if an instance exists', function(done) { INSTANCE.exists(function(err, exists) { assert.ifError(err); @@ -166,21 +149,6 @@ describe('Bigtable', function() { }); }); - it('should retrieve a list of clusters in stream mode', function(done) { - var clusters = []; - - INSTANCE.getClustersStream() - .on('error', done) - .on('data', function(cluster) { - assert(cluster instanceof Cluster); - clusters.push(cluster); - }) - .on('end', function() { - assert(clusters.length > 0); - done(); - }); - }); - it('should check if a cluster exists', function(done) { CLUSTER.exists(function(err, exists) { assert.ifError(err); diff --git a/test/index.js b/test/index.js index 7617a8366..3446a873d 100644 --- a/test/index.js +++ b/test/index.js @@ -68,15 +68,6 @@ function fakeRetryRequest() { ); } -var fakePaginator = { - extend: function() { - this.calledWith_ = arguments; - }, - streamify: function(methodName) { - return methodName; - }, -}; - function createFake(Class) { function Fake() { this.calledWith_ = arguments; @@ -98,7 +89,6 @@ describe('Bigtable', function() { before(function() { Bigtable = proxyquire('../', { '@google-cloud/common': { - paginator: fakePaginator, util: fakeUtil, }, 'google-auto-auth': fakeGoogleAutoAuth, @@ -141,17 +131,6 @@ describe('Bigtable', function() { } } - it('should extend the correct methods', function() { - var args = fakePaginator.calledWith_; - - assert.strictEqual(args[0], Bigtable); - assert.deepEqual(args[1], ['getInstances']); - }); - - it('should streamify the correct methods', function() { - assert.strictEqual(bigtable.getInstancesStream, 'getInstances'); - }); - it('should promisify all the things', function() { assert(promisified); }); @@ -484,8 +463,10 @@ describe('Bigtable', function() { bigtable.request = function(config) { assert.strictEqual(config.client, 'BigtableInstanceAdminClient'); assert.strictEqual(config.method, 'listInstances'); - assert.strictEqual(config.reqOpts.parent, bigtable.projectName); - assert.strictEqual(config.gaxOpts, undefined); + assert.deepStrictEqual(config.reqOpts, { + parent: bigtable.projectName, + }); + assert.deepEqual(config.gaxOpts, {}); done(); }; @@ -500,40 +481,18 @@ describe('Bigtable', function() { done(); }; - bigtable.getInstances({gaxOptions}, assert.ifError); - }); - - it('should copy all query options', function(done) { - var fakeOptions = { - a: 'a', - b: 'b', - }; - - bigtable.request = function(config) { - Object.keys(fakeOptions).forEach(function(key) { - assert.strictEqual(config.reqOpts[key], fakeOptions[key]); - }); - - assert.notStrictEqual(config.reqOpts, fakeOptions); - done(); - }; - - bigtable.getInstances(fakeOptions, assert.ifError); + bigtable.getInstances(gaxOptions, assert.ifError); }); it('should return an error to the callback', function(done) { var error = new Error('err'); - var response = {}; bigtable.request = function(config, callback) { - callback(error, response); + callback(error); }; - bigtable.getInstances(function(err, instances, nextQuery, apiResponse) { + bigtable.getInstances(function(err) { assert.strictEqual(err, error); - assert.strictEqual(instances, null); - assert.strictEqual(nextQuery, null); - assert.strictEqual(apiResponse, response); done(); }); }); @@ -550,13 +509,10 @@ describe('Bigtable', function() { ], }; - var responseArg2 = {}; - var responseArg3 = {}; - var fakeInstances = [{}, {}]; bigtable.request = function(config, callback) { - callback(null, response, responseArg2, responseArg3); + callback(null, response); }; var instanceCount = 0; @@ -566,42 +522,16 @@ describe('Bigtable', function() { return fakeInstances[instanceCount++]; }; - bigtable.getInstances(function(err, instances, nextQuery, apiResponse) { + bigtable.getInstances(function(err, instances, apiResponse) { assert.ifError(err); assert.strictEqual(instances[0], fakeInstances[0]); assert.strictEqual(instances[0].metadata, response.instances[0]); assert.strictEqual(instances[1], fakeInstances[1]); assert.strictEqual(instances[1].metadata, response.instances[1]); - assert.strictEqual(nextQuery, null); assert.strictEqual(apiResponse, response); done(); }); }); - - it('should provide a nextQuery object', function(done) { - var response = { - instances: [], - nextPageToken: 'a', - }; - - var options = { - a: 'b', - }; - - bigtable.request = function(config, callback) { - callback(null, response); - }; - - bigtable.getInstances(options, function(err, instances, nextQuery) { - var expectedQuery = extend({}, options, { - pageToken: response.nextPageToken, - }); - - assert.ifError(err); - assert.deepEqual(nextQuery, expectedQuery); - done(); - }); - }); }); describe('instance', function() { diff --git a/test/instance.js b/test/instance.js index 9280e9632..1e855d87c 100644 --- a/test/instance.js +++ b/test/instance.js @@ -97,11 +97,10 @@ describe('Bigtable/Instance', function() { var args = fakePaginator.calledWith_; assert.strictEqual(args[0], Instance); - assert.deepEqual(args[1], ['getClusters', 'getTables']); + assert.deepEqual(args[1], ['getTables']); }); it('should streamify the correct methods', function() { - assert.strictEqual(instance.getClustersStream, 'getClusters'); assert.strictEqual(instance.getTablesStream, 'getTables'); }); @@ -550,53 +549,51 @@ describe('Bigtable/Instance', function() { instance.bigtable.request = function(config) { assert.strictEqual(config.client, 'BigtableInstanceAdminClient'); assert.strictEqual(config.method, 'listClusters'); - assert.strictEqual(config.reqOpts.parent, INSTANCE_ID); - assert.strictEqual(config.gaxOpts, undefined); + assert.deepStrictEqual(config.reqOpts, { + parent: INSTANCE_ID, + }); + assert.deepEqual(config.gaxOpts, {}); done(); }; instance.getClusters(assert.ifError); }); - it('should copy all query options', function(done) { - var options = { - a: 'a', - b: 'b', - }; + it('should accept gaxOptions', function(done) { + var gaxOptions = {}; instance.bigtable.request = function(config) { - Object.keys(options).forEach(function(key) { - assert.strictEqual(config.reqOpts[key], options[key]); - }); - assert.notStrictEqual(config.reqOpts, options); + assert.strictEqual(config.gaxOpts, gaxOptions); done(); }; - instance.getClusters(options, assert.ifError); + instance.getClusters(gaxOptions, assert.ifError); }); - it('should accept gaxOptions', function(done) { - var options = { - gaxOptions: {}, - }; + it('should return error from gapic', function(done) { + var error = new Error('Error.'); - instance.bigtable.request = function(config) { - assert.strictEqual(config.gaxOpts, options.gaxOptions); - done(); + instance.bigtable.request = function(config, callback) { + callback(error); }; - instance.getClusters(options, assert.ifError); + instance.getClusters(function(err) { + assert.strictEqual(err, error); + done(); + }); }); it('should return an array of cluster objects', function(done) { - var response = [ - { - name: 'a', - }, - { - name: 'b', - }, - ]; + var response = { + clusters: [ + { + name: 'a', + }, + { + name: 'b', + }, + ], + }; var fakeClusters = [{}, {}]; @@ -607,31 +604,17 @@ describe('Bigtable/Instance', function() { var clusterCount = 0; instance.cluster = function(name) { - assert.strictEqual(name, response[clusterCount].name); + assert.strictEqual(name, response.clusters[clusterCount].name); return fakeClusters[clusterCount++]; }; - instance.getClusters(function(err, clusters) { + instance.getClusters(function(err, clusters, apiResponse) { assert.ifError(err); assert.strictEqual(clusters[0], fakeClusters[0]); - assert.strictEqual(clusters[0].metadata, response[0]); + assert.strictEqual(clusters[0].metadata, response.clusters[0]); assert.strictEqual(clusters[1], fakeClusters[1]); - assert.strictEqual(clusters[1].metadata, response[1]); - done(); - }); - }); - - it('should return original GAPIC response arguments', function(done) { - var response = [{}, null, {}, {}]; - - instance.bigtable.request = function(config, callback) { - callback.apply(null, response); - }; - - instance.getClusters(function() { - assert.strictEqual(arguments[0], response[0]); - assert.strictEqual(arguments[2], response[2]); - assert.strictEqual(arguments[3], response[3]); + assert.strictEqual(clusters[1].metadata, response.clusters[1]); + assert.strictEqual(apiResponse, response); done(); }); });