Skip to content

Commit

Permalink
fix: change exists to return false on error code 5 (#353)
Browse files Browse the repository at this point in the history
  • Loading branch information
callmehiphop authored and JustinBeckwith committed Oct 3, 2018
1 parent 3d47e54 commit 8a3be8b
Show file tree
Hide file tree
Showing 5 changed files with 207 additions and 73 deletions.
78 changes: 45 additions & 33 deletions src/database.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<DatabaseExistsResponse>}
*
* @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);
Expand Down Expand Up @@ -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<DatabaseExistsResponse>}
*
* @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}.
Expand Down
76 changes: 44 additions & 32 deletions src/instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<InstanceExistsResponse>}
*
* @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,
Expand Down Expand Up @@ -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<InstanceExistsResponse>}
*
* @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}.
Expand Down
32 changes: 32 additions & 0 deletions system-test/spanner.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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 (
Expand Down
47 changes: 43 additions & 4 deletions test/database.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down Expand Up @@ -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 = {};
Expand Down
47 changes: 43 additions & 4 deletions test/instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down Expand Up @@ -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 = {};
Expand Down

0 comments on commit 8a3be8b

Please sign in to comment.