From ef37be412400c4bd4be4372205328e9f78081589 Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Thu, 1 Mar 2018 11:24:07 -0500 Subject: [PATCH 1/3] Add autoCreate functionality. --- src/database.js | 126 ++++++++++++++++++------------ src/instance.js | 125 +++++++++++++++++++----------- system-test/spanner.js | 41 +++++++--- test/database.js | 170 ++++++++++++++++++++++++++++++++++++++++- test/instance.js | 170 ++++++++++++++++++++++++++++++++++++++++- 5 files changed, 527 insertions(+), 105 deletions(-) diff --git a/src/database.js b/src/database.js index 93cdb3f8d..af1685487 100644 --- a/src/database.js +++ b/src/database.js @@ -134,53 +134,6 @@ function Database(instance, name, poolOptions) { * }); */ exists: true, - - /** - * @typedef {array} GetDatabaseResponse - * @property {Database} 0 The {@link Database}. - * @property {object} 1 The full API response. - */ - /** - * @callback GetDatabaseCallback - * @param {?Error} err Request error, if any. - * @param {Database} database The {@link Database}. - * @param {object} apiResponse The full API response. - */ - /** - * Get a database if it exists. - * - * You may optionally use this to "get or create" an object by providing an - * object with `autoCreate` set to `true`. Any extra configuration that is - * normally required for the `create` method must be contained within this - * object as well. - * - * @method Database#get - * @param {options} [options] Configuration object. - * @param {boolean} [options.autoCreate=false] Automatically create the - * object if it does not exist. - * @param {GetDatabaseCallback} [callback] Callback function. - * @returns {Promise} - * - * @example - * const Spanner = require('@google-cloud/spanner'); - * const spanner = new Spanner(); - * - * const instance = spanner.instance('my-instance'); - * const database = instance.database('my-database'); - * - * database.get(function(err, database, apiResponse) { - * // `database.metadata` has been populated. - * }); - * - * //- - * // If the callback is omitted, we'll return a Promise. - * //- - * database.get().then(function(data) { - * var database = data[0]; - * var apiResponse = data[0]; - * }); - */ - get: true, }; commonGrpc.ServiceObject.call(this, { @@ -492,6 +445,85 @@ Database.prototype.delete = function(callback) { }); }; +/** + * @typedef {array} GetDatabaseResponse + * @property {Database} 0 The {@link Database}. + * @property {object} 1 The full API response. + */ +/** + * @callback GetDatabaseCallback + * @param {?Error} err Request error, if any. + * @param {Database} database The {@link Database}. + * @param {object} apiResponse The full API response. + */ +/** + * Get a database if it exists. + * + * You may optionally use this to "get or create" an object by providing an + * object with `autoCreate` set to `true`. Any extra configuration that is + * normally required for the `create` method must be contained within this + * object as well. + * + * @param {options} [options] Configuration object. + * @param {boolean} [options.autoCreate=false] Automatically create the + * object if it does not exist. + * @param {GetDatabaseCallback} [callback] Callback function. + * @returns {Promise} + * + * @example + * const Spanner = require('@google-cloud/spanner'); + * const spanner = new Spanner(); + * + * const instance = spanner.instance('my-instance'); + * const database = instance.database('my-database'); + * + * database.get(function(err, database, apiResponse) { + * // `database.metadata` has been populated. + * }); + * + * //- + * // If the callback is omitted, we'll return a Promise. + * //- + * database.get().then(function(data) { + * var database = data[0]; + * var apiResponse = data[0]; + * }); + */ +Database.prototype.get = function(options, callback) { + var self = this; + + if (is.fn(options)) { + callback = options; + options = {}; + } + + this.getMetadata(function(err, metadata) { + if (err) { + if (options.autoCreate && err.code === 5) { + self.create(options, function(err, database, operation) { + if (err) { + callback(err); + return; + } + + operation + .on('error', callback) + .on('complete', function(metadata) { + self.metadata = metadata; + callback(null, self, metadata); + }); + }); + return; + } + + callback(err); + return; + } + + callback(null, self, metadata); + }); +}; + /** * @typedef {array} GetDatabaseMetadataResponse * @property {object} 0 The {@link Database} metadata. diff --git a/src/instance.js b/src/instance.js index dbc05fd90..cdbbc1008 100644 --- a/src/instance.js +++ b/src/instance.js @@ -128,52 +128,6 @@ function Instance(spanner, name) { * }); */ exists: true, - - /** - * @typedef {array} GetInstanceResponse - * @property {Instance} 0 The {@link Instance}. - * @property {object} 1 The full API response. - */ - /** - * @callback GetInstanceCallback - * @param {?Error} err Request error, if any. - * @param {Instance} instance The {@link Instance}. - * @param {object} apiResponse The full API response. - */ - /** - * Get an instance if it exists. - * - * You may optionally use this to "get or create" an object by providing an - * object with `autoCreate` set to `true`. Any extra configuration that is - * normally required for the `create` method must be contained within this - * object as well. - * - * @method Instance#get - * @param {options} [options] Configuration object. - * @param {boolean} [options.autoCreate=false] Automatically create the - * object if it does not exist. - * @param {GetInstanceCallback} [callback] Callback function. - * @returns {Promise} - * - * @example - * const Spanner = require('@google-cloud/spanner'); - * const spanner = new Spanner(); - * - * const instance = spanner.instance('my-instance'); - * - * instance.get(function(err, instance, apiResponse) { - * // `instance.metadata` has been populated. - * }); - * - * //- - * // If the callback is omitted, we'll return a Promise. - * //- - * instance.get().then(function(data) { - * var instance = data[0]; - * var apiResponse = data[0]; - * }); - */ - get: true, }; commonGrpc.ServiceObject.call(this, { @@ -453,6 +407,85 @@ Instance.prototype.delete = function(callback) { }); }; + +/** + * @typedef {array} GetInstanceResponse + * @property {Instance} 0 The {@link Instance}. + * @property {object} 1 The full API response. + */ +/** + * @callback GetInstanceCallback + * @param {?Error} err Request error, if any. + * @param {Instance} instance The {@link Instance}. + * @param {object} apiResponse The full API response. + */ +/** + * Get an instance if it exists. + * + * You may optionally use this to "get or create" an object by providing an + * object with `autoCreate` set to `true`. Any extra configuration that is + * normally required for the `create` method must be contained within this + * object as well. + * + * @param {options} [options] Configuration object. + * @param {boolean} [options.autoCreate=false] Automatically create the + * object if it does not exist. + * @param {GetInstanceCallback} [callback] Callback function. + * @returns {Promise} + * + * @example + * const Spanner = require('@google-cloud/spanner'); + * const spanner = new Spanner(); + * + * const instance = spanner.instance('my-instance'); + * + * instance.get(function(err, instance, apiResponse) { + * // `instance.metadata` has been populated. + * }); + * + * //- + * // If the callback is omitted, we'll return a Promise. + * //- + * instance.get().then(function(data) { + * var instance = data[0]; + * var apiResponse = data[0]; + * }); + */ +Instance.prototype.get = function(options, callback) { + var self = this; + + if (is.fn(options)) { + callback = options; + options = {}; + } + + this.getMetadata(function(err, metadata) { + if (err) { + if (err.code === 5 && options.autoCreate) { + self.create(options, function(err, database, operation) { + if (err) { + callback(err); + return; + } + + operation + .on('error', callback) + .on('complete', function(metadata) { + self.metadata = metadata; + callback(null, self, metadata); + }); + }); + return; + } + + callback(err); + return; + } + + callback(null, self, metadata); + }); +}; + /** * Query object for listing databases. * diff --git a/system-test/spanner.js b/system-test/spanner.js index fb764f1e4..395cecbfb 100644 --- a/system-test/spanner.js +++ b/system-test/spanner.js @@ -34,22 +34,21 @@ var spanner = new Spanner({projectId: process.env.GCLOUD_PROJECT}); describe('Spanner', function() { var instance = spanner.instance(generateName('instance')); + var INSTANCE_CONFIG = { + config: 'regional-us-central1', + nodes: 1, + labels: { + 'gcloud-tests': 'true', + }, + }; + before(function(done) { async.series( [ deleteTestResources, function(next) { - instance.create( - { - config: 'regional-us-central1', - nodes: 1, - labels: { - 'gcloud-tests': 'true', - }, - }, - execAfterOperationComplete(next) - ); + instance.create(INSTANCE_CONFIG, execAfterOperationComplete(next)); }, ], done @@ -618,6 +617,19 @@ describe('Spanner', function() { }); }); + it('should auto create an instance', function(done) { + var instance = spanner.instance(generateName('instance')); + + var config = extend({ + autoCreate: true, + }, INSTANCE_CONFIG); + + instance.get(config, function(err) { + assert.ifError(err); + instance.getMetadata(done); + }); + }); + it('should list the instances', function(done) { spanner.getInstances(function(err, instances) { assert.ifError(err); @@ -719,6 +731,15 @@ describe('Spanner', function() { }); }); + it('should auto create a database', function(done) { + var database = instance.database(generateName('database')); + + database.get({ autoCreate: true }, function(err) { + assert.ifError(err); + database.getMetadata(done); + }); + }); + it('should have created the database', function(done) { database.getMetadata(function(err, metadata) { assert.ifError(err); diff --git a/test/database.js b/test/database.js index ace8cd636..d5812de8b 100644 --- a/test/database.js +++ b/test/database.js @@ -212,7 +212,6 @@ describe('Database', function() { assert.deepEqual(calledWith.methods, { create: true, exists: true, - get: true, }); calledWith.createMethod(null, options, done); @@ -532,6 +531,175 @@ describe('Database', function() { }); }); + describe('get', function() { + it('should call getMetadata', function(done) { + var options = {}; + + database.getMetadata = function() { + done(); + }; + + database.get(options, assert.ifError); + }); + + it('should not require an options object', function(done) { + database.getMetadata = function() { + done(); + }; + + database.get(assert.ifError); + }); + + describe('autoCreate', function() { + var error = new Error('Error.'); + error.code = 5; + + var OPTIONS = { + autoCreate: true, + }; + + var OPERATION = { + listeners: {}, + on: function(eventName, callback) { + OPERATION.listeners[eventName] = callback; + return OPERATION; + }, + }; + + beforeEach(function() { + OPERATION.listeners = {}; + + database.getMetadata = function(callback) { + callback(error); + }; + + database.create = function(options, callback) { + callback(null, null, OPERATION); + }; + }); + + it('should call create', function(done) { + database.create = function(options, callback) { + assert.strictEqual(options, OPTIONS); + done(); + }; + + database.get(OPTIONS, assert.ifError); + }); + + it('should return error if create failed', function(done) { + var error = new Error('Error.'); + + database.create = function(options, callback) { + callback(error); + }; + + database.get(OPTIONS, function(err) { + assert.strictEqual(err, error); + done(); + }); + }); + + it('should return operation error', function(done) { + var error = new Error('Error.'); + + setImmediate(function() { + OPERATION.listeners['error'](error); + }); + + database.get(OPTIONS, function(err) { + assert.strictEqual(err, error); + done(); + }); + }); + + it('should execute callback if opereation succeeded', function(done) { + var error = new Error('Error.'); + var metadata = {}; + + setImmediate(function() { + OPERATION.listeners['complete'](metadata); + }); + + database.get(OPTIONS, function(err, database_, apiResponse) { + assert.ifError(err); + assert.strictEqual(database_, database); + assert.strictEqual(database.metadata, metadata); + assert.strictEqual(metadata, apiResponse); + done(); + }); + }); + }); + + it('should not auto create without error code 5', function(done) { + var error = new Error('Error.'); + error.code = 'NOT-5'; + + var options = { + autoCreate: true, + }; + + database.getMetadata = function(callback) { + callback(error); + }; + + database.create = function() { + throw new Error('Should not create.'); + }; + + database.get(options, function(err) { + assert.strictEqual(err, error); + done(); + }); + }); + + it('should not auto create unless requested', function(done) { + var error = new Error('Error.'); + error.code = 5; + + database.getMetadata = function(callback) { + callback(error); + }; + + database.create = function() { + throw new Error('Should not create.'); + }; + + database.get(function(err) { + assert.strictEqual(err, error); + done(); + }); + }); + + it('should return an error from getMetadata', function(done) { + var error = new Error('Error.'); + + database.getMetadata = function(callback) { + callback(error); + }; + + database.get(function(err) { + assert.strictEqual(err, error); + done(); + }); + }); + + it('should return self and API response', function(done) { + var apiResponse = {}; + + database.getMetadata = function(callback) { + callback(null, apiResponse); + }; + + database.get(function(err, database_, apiResponse_) { + assert.ifError(err); + assert.strictEqual(database_, database); + assert.strictEqual(apiResponse_, apiResponse); + done(); + }); + }); + }); + describe('getMetadata', function() { it('should call and return the request', function() { var requestReturnValue = {}; diff --git a/test/instance.js b/test/instance.js index 6a8705cae..a3d03909f 100644 --- a/test/instance.js +++ b/test/instance.js @@ -149,7 +149,6 @@ describe('Instance', function() { assert.deepEqual(calledWith.methods, { create: true, exists: true, - get: true, }); calledWith.createMethod(null, options, done); @@ -431,6 +430,175 @@ describe('Instance', function() { }); }); + describe('get', function() { + it('should call getMetadata', function(done) { + var options = {}; + + instance.getMetadata = function() { + done(); + }; + + instance.get(options, assert.ifError); + }); + + it('should not require an options object', function(done) { + instance.getMetadata = function() { + done(); + }; + + instance.get(assert.ifError); + }); + + describe('autoCreate', function() { + var error = new Error('Error.'); + error.code = 5; + + var OPTIONS = { + autoCreate: true, + }; + + var OPERATION = { + listeners: {}, + on: function(eventName, callback) { + OPERATION.listeners[eventName] = callback; + return OPERATION; + }, + }; + + beforeEach(function() { + OPERATION.listeners = {}; + + instance.getMetadata = function(callback) { + callback(error); + }; + + instance.create = function(options, callback) { + callback(null, null, OPERATION); + }; + }); + + it('should call create', function(done) { + instance.create = function(options, callback) { + assert.strictEqual(options, OPTIONS); + done(); + }; + + instance.get(OPTIONS, assert.ifError); + }); + + it('should return error if create failed', function(done) { + var error = new Error('Error.'); + + instance.create = function(options, callback) { + callback(error); + }; + + instance.get(OPTIONS, function(err) { + assert.strictEqual(err, error); + done(); + }); + }); + + it('should return operation error', function(done) { + var error = new Error('Error.'); + + setImmediate(function() { + OPERATION.listeners['error'](error); + }); + + instance.get(OPTIONS, function(err) { + assert.strictEqual(err, error); + done(); + }); + }); + + it('should execute callback if opereation succeeded', function(done) { + var error = new Error('Error.'); + var metadata = {}; + + setImmediate(function() { + OPERATION.listeners['complete'](metadata); + }); + + instance.get(OPTIONS, function(err, instance_, apiResponse) { + assert.ifError(err); + assert.strictEqual(instance_, instance); + assert.strictEqual(instance.metadata, metadata); + assert.strictEqual(metadata, apiResponse); + done(); + }); + }); + }); + + it('should not auto create without error code 5', function(done) { + var error = new Error('Error.'); + error.code = 'NOT-5'; + + var options = { + autoCreate: true, + }; + + instance.getMetadata = function(callback) { + callback(error); + }; + + instance.create = function() { + throw new Error('Should not create.'); + }; + + instance.get(options, function(err) { + assert.strictEqual(err, error); + done(); + }); + }); + + it('should not auto create unless requested', function(done) { + var error = new Error('Error.'); + error.code = 5; + + instance.getMetadata = function(callback) { + callback(error); + }; + + instance.create = function() { + throw new Error('Should not create.'); + }; + + instance.get(function(err) { + assert.strictEqual(err, error); + done(); + }); + }); + + it('should return an error from getMetadata', function(done) { + var error = new Error('Error.'); + + instance.getMetadata = function(callback) { + callback(error); + }; + + instance.get(function(err) { + assert.strictEqual(err, error); + done(); + }); + }); + + it('should return self and API response', function(done) { + var apiResponse = {}; + + instance.getMetadata = function(callback) { + callback(null, apiResponse); + }; + + instance.get(function(err, instance_, apiResponse_) { + assert.ifError(err); + assert.strictEqual(instance_, instance); + assert.strictEqual(apiResponse_, apiResponse); + done(); + }); + }); + }); + describe('getDatabases', function() { var QUERY = { a: 'b', From d6dea55ad42c9a5da943636c85dd481856ab947f Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Thu, 1 Mar 2018 12:08:02 -0500 Subject: [PATCH 2/3] Prettier --- src/database.js | 10 ++++------ src/instance.js | 11 ++++------- system-test/spanner.js | 11 +++++++---- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/database.js b/src/database.js index af1685487..30c14bf4a 100644 --- a/src/database.js +++ b/src/database.js @@ -506,12 +506,10 @@ Database.prototype.get = function(options, callback) { return; } - operation - .on('error', callback) - .on('complete', function(metadata) { - self.metadata = metadata; - callback(null, self, metadata); - }); + operation.on('error', callback).on('complete', function(metadata) { + self.metadata = metadata; + callback(null, self, metadata); + }); }); return; } diff --git a/src/instance.js b/src/instance.js index cdbbc1008..36303fa04 100644 --- a/src/instance.js +++ b/src/instance.js @@ -407,7 +407,6 @@ Instance.prototype.delete = function(callback) { }); }; - /** * @typedef {array} GetInstanceResponse * @property {Instance} 0 The {@link Instance}. @@ -468,12 +467,10 @@ Instance.prototype.get = function(options, callback) { return; } - operation - .on('error', callback) - .on('complete', function(metadata) { - self.metadata = metadata; - callback(null, self, metadata); - }); + operation.on('error', callback).on('complete', function(metadata) { + self.metadata = metadata; + callback(null, self, metadata); + }); }); return; } diff --git a/system-test/spanner.js b/system-test/spanner.js index 395cecbfb..e336e5e68 100644 --- a/system-test/spanner.js +++ b/system-test/spanner.js @@ -620,9 +620,12 @@ describe('Spanner', function() { it('should auto create an instance', function(done) { var instance = spanner.instance(generateName('instance')); - var config = extend({ - autoCreate: true, - }, INSTANCE_CONFIG); + var config = extend( + { + autoCreate: true, + }, + INSTANCE_CONFIG + ); instance.get(config, function(err) { assert.ifError(err); @@ -734,7 +737,7 @@ describe('Spanner', function() { it('should auto create a database', function(done) { var database = instance.database(generateName('database')); - database.get({ autoCreate: true }, function(err) { + database.get({autoCreate: true}, function(err) { assert.ifError(err); database.getMetadata(done); }); From ca34a9b14327ee986d0b9d4b41c06972acbc761b Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Thu, 1 Mar 2018 12:09:18 -0500 Subject: [PATCH 3/3] Lint --- test/database.js | 3 +-- test/instance.js | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/test/database.js b/test/database.js index d5812de8b..7f2e0c7ce 100644 --- a/test/database.js +++ b/test/database.js @@ -579,7 +579,7 @@ describe('Database', function() { }); it('should call create', function(done) { - database.create = function(options, callback) { + database.create = function(options) { assert.strictEqual(options, OPTIONS); done(); }; @@ -614,7 +614,6 @@ describe('Database', function() { }); it('should execute callback if opereation succeeded', function(done) { - var error = new Error('Error.'); var metadata = {}; setImmediate(function() { diff --git a/test/instance.js b/test/instance.js index a3d03909f..7ec931360 100644 --- a/test/instance.js +++ b/test/instance.js @@ -478,7 +478,7 @@ describe('Instance', function() { }); it('should call create', function(done) { - instance.create = function(options, callback) { + instance.create = function(options) { assert.strictEqual(options, OPTIONS); done(); }; @@ -513,7 +513,6 @@ describe('Instance', function() { }); it('should execute callback if opereation succeeded', function(done) { - var error = new Error('Error.'); var metadata = {}; setImmediate(function() {