From 8a3be8b5f6419be9617c4bf759a48ba568d185e2 Mon Sep 17 00:00:00 2001 From: Dave Gramlich Date: Wed, 3 Oct 2018 00:29:13 -0400 Subject: [PATCH] fix: change exists to return false on error code 5 (#353) --- src/database.js | 78 ++++++++++++++++++++++++------------------ src/instance.js | 76 +++++++++++++++++++++++----------------- system-test/spanner.js | 32 +++++++++++++++++ test/database.js | 47 ++++++++++++++++++++++--- test/instance.js | 47 ++++++++++++++++++++++--- 5 files changed, 207 insertions(+), 73 deletions(-) diff --git a/src/database.js b/src/database.js index e617e1384..9aafba1e8 100644 --- a/src/database.js +++ b/src/database.js @@ -146,39 +146,6 @@ class Database extends ServiceObject { * }); */ create: true, - /** - * @typedef {array} DatabaseExistsResponse - * @property {boolean} 0 Whether the {@link Database} exists. - */ - /** - * @callback DatabaseExistsCallback - * @param {?Error} err Request error, if any. - * @param {boolean} exists Whether the {@link Database} exists. - */ - /** - * Check if a database exists. - * - * @method Database#exists - * @param {DatabaseExistsCallback} [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.exists(function(err, exists) {}); - * - * //- - * // If the callback is omitted, we'll return a Promise. - * //- - * database.exists().then(function(data) { - * const exists = data[0]; - * }); - */ - exists: true, }; const formattedName_ = Database.formatName_(instance.formattedName_, name); @@ -547,6 +514,51 @@ class Database extends ServiceObject { ); }); } + /** + * @typedef {array} DatabaseExistsResponse + * @property {boolean} 0 Whether the {@link Database} exists. + */ + /** + * @callback DatabaseExistsCallback + * @param {?Error} err Request error, if any. + * @param {boolean} exists Whether the {@link Database} exists. + */ + /** + * Check if a database exists. + * + * @method Database#exists + * @param {DatabaseExistsCallback} [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.exists(function(err, exists) {}); + * + * //- + * // If the callback is omitted, we'll return a Promise. + * //- + * database.exists().then(function(data) { + * const exists = data[0]; + * }); + */ + exists(callback) { + const NOT_FOUND = 5; + + this.getMetadata(function(err) { + if (err && err.code !== NOT_FOUND) { + callback(err, null); + return; + } + + const exists = !err || err.code !== NOT_FOUND; + callback(null, exists); + }); + } /** * @typedef {array} GetDatabaseResponse * @property {Database} 0 The {@link Database}. diff --git a/src/instance.js b/src/instance.js index 473048476..934f624d3 100644 --- a/src/instance.js +++ b/src/instance.js @@ -91,38 +91,6 @@ class Instance extends common.ServiceObject { * }); */ create: true, - /** - * @typedef {array} InstanceExistsResponse - * @property {boolean} 0 Whether the {@link Instance} exists. - */ - /** - * @callback InstanceExistsCallback - * @param {?Error} err Request error, if any. - * @param {boolean} exists Whether the {@link Instance} exists. - */ - /** - * Check if an instance exists. - * - * @method Instance#exists - * @param {InstanceExistsCallback} [callback] Callback function. - * @returns {Promise} - * - * @example - * const {Spanner} = require('@google-cloud/spanner'); - * const spanner = new Spanner(); - * - * const instance = spanner.instance('my-instance'); - * - * instance.exists(function(err, exists) {}); - * - * //- - * // If the callback is omitted, we'll return a Promise. - * //- - * instance.exists().then(function(data) { - * const exists = data[0]; - * }); - */ - exists: true, }; super({ parent: spanner, @@ -363,6 +331,50 @@ class Instance extends common.ServiceObject { ); }); } + /** + * @typedef {array} InstanceExistsResponse + * @property {boolean} 0 Whether the {@link Instance} exists. + */ + /** + * @callback InstanceExistsCallback + * @param {?Error} err Request error, if any. + * @param {boolean} exists Whether the {@link Instance} exists. + */ + /** + * Check if an instance exists. + * + * @method Instance#exists + * @param {InstanceExistsCallback} [callback] Callback function. + * @returns {Promise} + * + * @example + * const {Spanner} = require('@google-cloud/spanner'); + * const spanner = new Spanner(); + * + * const instance = spanner.instance('my-instance'); + * + * instance.exists(function(err, exists) {}); + * + * //- + * // If the callback is omitted, we'll return a Promise. + * //- + * instance.exists().then(function(data) { + * const exists = data[0]; + * }); + */ + exists(callback) { + const NOT_FOUND = 5; + + this.getMetadata(function(err) { + if (err && err.code !== NOT_FOUND) { + callback(err, null); + return; + } + + const exists = !err || err.code !== NOT_FOUND; + callback(null, exists); + }); + } /** * @typedef {array} GetInstanceResponse * @property {Instance} 0 The {@link Instance}. diff --git a/system-test/spanner.js b/system-test/spanner.js index d7337aa39..f74713a68 100644 --- a/system-test/spanner.js +++ b/system-test/spanner.js @@ -733,6 +733,22 @@ describe('Spanner', function() { }) ); }); + + it('should return true for instances that exist', function(done) { + instance.exists(function(err, exists) { + assert.ifError(err); + assert.strictEqual(exists, true); + done(); + }); + }); + + it('should return false for instances that do not exist', function(done) { + spanner.instance('bad-instance').exists(function(err, exists) { + assert.ifError(err); + assert.strictEqual(exists, false); + done(); + }); + }); }); describe('instanceConfigs', function() { @@ -834,6 +850,22 @@ describe('Spanner', function() { ); }); + it('should return true for databases that exist', function(done) { + database.exists(function(err, exists) { + assert.ifError(err); + assert.strictEqual(exists, true); + done(); + }); + }); + + it('should return false for databases that do not exist', function(done) { + instance.database('bad-database').exists(function(err, exists) { + assert.ifError(err); + assert.strictEqual(exists, false); + done(); + }); + }); + it('should create a table', function(done) { const createTableStatement = ` CREATE TABLE Singers ( diff --git a/test/database.js b/test/database.js index 189a834c4..4eca8fcee 100644 --- a/test/database.js +++ b/test/database.js @@ -209,10 +209,7 @@ describe('Database', function() { assert.strictEqual(calledWith.parent, instanceInstance); assert.strictEqual(calledWith.id, NAME); - assert.deepStrictEqual(calledWith.methods, { - create: true, - exists: true, - }); + assert.deepStrictEqual(calledWith.methods, {create: true}); calledWith.createMethod(null, options, done); }); @@ -550,6 +547,48 @@ describe('Database', function() { }); }); + describe('exists', function() { + it('should return any non-404 like errors', function(done) { + const error = {code: 3}; + + database.getMetadata = function(callback) { + callback(error); + }; + + database.exists(function(err, exists) { + assert.strictEqual(err, error); + assert.strictEqual(exists, null); + done(); + }); + }); + + it('should return true if error is absent', function(done) { + database.getMetadata = function(callback) { + callback(null); + }; + + database.exists(function(err, exists) { + assert.ifError(err); + assert.strictEqual(exists, true); + done(); + }); + }); + + it('should return false if not found error if present', function(done) { + const error = {code: 5}; + + database.getMetadata = function(callback) { + callback(error); + }; + + database.exists(function(err, exists) { + assert.ifError(err); + assert.strictEqual(exists, false); + done(); + }); + }); + }); + describe('get', function() { it('should call getMetadata', function(done) { const options = {}; diff --git a/test/instance.js b/test/instance.js index 9996c3695..4d50f05a7 100644 --- a/test/instance.js +++ b/test/instance.js @@ -144,10 +144,7 @@ describe('Instance', function() { assert.strictEqual(calledWith.parent, spannerInstance); assert.strictEqual(calledWith.id, NAME); - assert.deepStrictEqual(calledWith.methods, { - create: true, - exists: true, - }); + assert.deepStrictEqual(calledWith.methods, {create: true}); calledWith.createMethod(null, options, done); }); @@ -428,6 +425,48 @@ describe('Instance', function() { }); }); + describe('exists', function() { + it('should return any non-404 like errors', function(done) { + const error = {code: 3}; + + instance.getMetadata = function(callback) { + callback(error); + }; + + instance.exists(function(err, exists) { + assert.strictEqual(err, error); + assert.strictEqual(exists, null); + done(); + }); + }); + + it('should return true if error is absent', function(done) { + instance.getMetadata = function(callback) { + callback(null); + }; + + instance.exists(function(err, exists) { + assert.ifError(err); + assert.strictEqual(exists, true); + done(); + }); + }); + + it('should return false if not found error if present', function(done) { + const error = {code: 5}; + + instance.getMetadata = function(callback) { + callback(error); + }; + + instance.exists(function(err, exists) { + assert.ifError(err); + assert.strictEqual(exists, false); + done(); + }); + }); + }); + describe('get', function() { it('should call getMetadata', function(done) { const options = {};