From 7f6a2c89615ef03199c9b5b14c4451a88142ece6 Mon Sep 17 00:00:00 2001 From: Jason Dobry Date: Tue, 15 Nov 2016 16:39:16 -0800 Subject: [PATCH] Addressed comments. --- datastore/concepts.js | 372 ++++++++++++------------- datastore/system-test/concepts.test.js | 2 +- datastore/tasks.js | 24 +- 3 files changed, 191 insertions(+), 207 deletions(-) diff --git a/datastore/concepts.js b/datastore/concepts.js index 73790bcc6c..ccf1f79a06 100644 --- a/datastore/concepts.js +++ b/datastore/concepts.js @@ -22,16 +22,20 @@ const assert = require('power-assert'); // https://googlecloudplatform.github.io/gcloud-node/#/docs/google-cloud/latest/guides/authentication const Datastore = require('@google-cloud/datastore'); +function makeStub () { + return sinon.stub().returns(Promise.resolve([])); +} + // This mock is used in the documentation snippets. let datastore = { - delete: sinon.stub().returns(Promise.resolve([])), - get: sinon.stub().returns(Promise.resolve([])), - insert: sinon.stub().returns(Promise.resolve([])), - key: sinon.stub().returns(Promise.resolve([])), - update: sinon.stub().returns(Promise.resolve([])), - upsert: sinon.stub().returns(Promise.resolve([])), + delete: makeStub(), + get: makeStub(), + insert: makeStub(), + key: makeStub(), + update: makeStub(), + upsert: makeStub(), runQuery: sinon.stub().returns(Promise.resolve([[]])), - save: sinon.stub().returns(Promise.resolve([])) + save: makeStub() }; class TestHelper { @@ -279,7 +283,7 @@ class Entity extends TestHelper { // Task found. const entity = results[0]; - // entity.data = { + // entity = { // category: 'Personal', // done: false, // priority: 4, @@ -974,6 +978,7 @@ class Query extends TestHelper { // the `endCursor` property. return runPageQuery(info.endCursor) .then((results) => { + // Concatenate entities results[0] = entities.concat(results[0]); return results; }); @@ -1032,210 +1037,203 @@ function transferFunds (fromKey, toKey, amount) { } // [END transactional_update] -function Transaction (projectId) { - var options = { - projectId: projectId - }; - - this.datastore = Datastore(options); - - this.fromKey = this.datastore.key(['Bank', 1, 'Account', 1]); - this.toKey = this.datastore.key(['Bank', 1, 'Account', 2]); +class Transaction extends TestHelper { + constructor (projectId) { + super(projectId); + this.fromKey = this.datastore.key(['Bank', 1, 'Account', 1]); + this.toKey = this.datastore.key(['Bank', 1, 'Account', 2]); - this.originalBalance = 100; - this.amountToTransfer = 10; -} + this.originalBalance = 100; + this.amountToTransfer = 10; + } -Transaction.prototype.restoreBankAccountBalances = function (config, callback) { - const entities = config.keys.map((key) => { - return { - key: key, - data: { - balance: config.balance - } - }; - }); + restoreBankAccountBalances (config) { + const entities = config.keys.map((key) => { + return { + key: key, + data: { + balance: config.balance + } + }; + }); - if (callback) { - this.datastore.save(entities, callback); - } else { return this.datastore.save(entities); } -}; -Transaction.prototype.testTransactionalUpdate = function () { - const fromKey = this.fromKey; - const toKey = this.toKey; - const originalBalance = this.originalBalance; - const amountToTransfer = this.amountToTransfer; - const datastoreMock = datastore; - - // Overwrite so the real Datastore instance is used in `transferFunds`. - datastore = this.datastore; - - return this.restoreBankAccountBalances({ - keys: [fromKey, toKey], - balance: originalBalance - }) - .then(() => transferFunds(fromKey, toKey, amountToTransfer)) - .then(() => Promise.all([this.datastore.get(fromKey), this.datastore.get(toKey)])) - .then((results) => { - const accounts = results.map((result) => result[0]); - // Restore `datastore` to the mock API. - datastore = datastoreMock; - assert.equal(accounts[0].balance, originalBalance - amountToTransfer); - assert.equal(accounts[1].balance, originalBalance + amountToTransfer); - }) - .catch((err) => { - // Restore `datastore` to the mock API. - datastore = datastoreMock; - return Promise.reject(err); - }); -}; + testTransactionalUpdate () { + const fromKey = this.fromKey; + const toKey = this.toKey; + const originalBalance = this.originalBalance; + const amountToTransfer = this.amountToTransfer; + const datastoreMock = datastore; -Transaction.prototype.testTransactionalRetry = function () { - // Overwrite so the real Datastore instance is used in `transferFunds`. - const datastoreMock = datastore; - datastore = this.datastore; - - const fromKey = this.fromKey; - const toKey = this.toKey; - - return this.restoreBankAccountBalances({ - keys: [fromKey, toKey], - balance: this.originalBalance - }) - .then(() => { - // [START transactional_retry] - function transferFundsWithRetry () { - const maxTries = 5; - let currentAttempt = 1; - let delay = 100; - - function tryRequest () { - return transferFunds(fromKey, toKey, 10) - .catch((err) => { - if (currentAttempt <= maxTries) { - // Use exponential backoff - return new Promise((resolve, reject) => { - setTimeout(() => { - currentAttempt++; - delay *= 2; - tryRequest().then(resolve, reject); - }, delay); - }); - } - return Promise.reject(err); - }); - } + // Overwrite so the real Datastore instance is used in `transferFunds`. + datastore = this.datastore; - return tryRequest(1, 5); - } - // [END transactional_retry] - return transferFundsWithRetry(); + return this.restoreBankAccountBalances({ + keys: [fromKey, toKey], + balance: originalBalance }) - .then(() => { - // Restore `datastore` to the mock API. - datastore = datastoreMock; - }) - .catch(() => { - // Restore `datastore` to the mock API. - datastore = datastoreMock; - }); -}; - -Transaction.prototype.testTransactionalGetOrCreate = function () { - const taskKey = this.datastore.key(['Task', Date.now()]); + .then(() => transferFunds(fromKey, toKey, amountToTransfer)) + .then(() => Promise.all([this.datastore.get(fromKey), this.datastore.get(toKey)])) + .then((results) => { + const accounts = results.map((result) => result[0]); + // Restore `datastore` to the mock API. + datastore = datastoreMock; + assert.equal(accounts[0].balance, originalBalance - amountToTransfer); + assert.equal(accounts[1].balance, originalBalance + amountToTransfer); + }) + .catch((err) => { + // Restore `datastore` to the mock API. + datastore = datastoreMock; + return Promise.reject(err); + }); + } - // Overwrite so the real Datastore instance is used in `transferFunds`. - const datastoreMock = datastore; - datastore = this.datastore; + testTransactionalRetry () { + // Overwrite so the real Datastore instance is used in `transferFunds`. + const datastoreMock = datastore; + datastore = this.datastore; - // [START transactional_get_or_create] - function getOrCreate (taskKey, taskData) { - const taskEntity = { - key: taskKey, - data: taskData - }; + const fromKey = this.fromKey; + const toKey = this.toKey; - const transaction = datastore.transaction(); + return this.restoreBankAccountBalances({ + keys: [fromKey, toKey], + balance: this.originalBalance + }) + .then(() => { + // [START transactional_retry] + function transferFundsWithRetry () { + const maxTries = 5; + let currentAttempt = 1; + let delay = 100; + + function tryRequest () { + return transferFunds(fromKey, toKey, 10) + .catch((err) => { + if (currentAttempt <= maxTries) { + // Use exponential backoff + return new Promise((resolve, reject) => { + setTimeout(() => { + currentAttempt++; + delay *= 2; + tryRequest().then(resolve, reject); + }, delay); + }); + } + return Promise.reject(err); + }); + } - return transaction.run() - .then(() => transaction.get(taskKey)) - .then((results) => { - const task = results[0]; - if (task) { - // The task entity already exists. - return transaction.rollback(); - } else { - // Create the task entity. - transaction.save(taskEntity); - return transaction.commit(); + return tryRequest(1, 5); } + // [END transactional_retry] + return transferFundsWithRetry(); }) - .then(() => taskEntity) - .catch(() => transaction.rollback()); + .then(() => { + // Restore `datastore` to the mock API. + datastore = datastoreMock; + }) + .catch(() => { + // Restore `datastore` to the mock API. + datastore = datastoreMock; + }); } - // [END transactional_get_or_create] - return getOrCreate(taskKey, {}) - .then((task) => { - assert(task, 'Should have a task.'); - return getOrCreate(taskKey, {}); - }) - .then((task) => { - assert(task, 'Should have a task.'); - // Restore `datastore` to the mock API. - datastore = datastoreMock; - }) - .catch((err) => { - // Restore `datastore` to the mock API. - datastore = datastoreMock; - return Promise.reject(err); - }); -}; + testTransactionalGetOrCreate () { + const taskKey = this.datastore.key(['Task', Date.now()]); -Transaction.prototype.testSingleEntityGroupReadOnly = function () { - // Overwrite so the real Datastore instance is used in `transferFunds`. - const datastoreMock = datastore; - datastore = this.datastore; + // Overwrite so the real Datastore instance is used in `transferFunds`. + const datastoreMock = datastore; + datastore = this.datastore; - // [START transactional_single_entity_group_read_only] - function getTaskListEntities () { - let taskList, taskListEntities; + // [START transactional_get_or_create] + function getOrCreate (taskKey, taskData) { + const taskEntity = { + key: taskKey, + data: taskData + }; - const transaction = datastore.transaction(); - const taskListKey = datastore.key(['TaskList', 'default']); + const transaction = datastore.transaction(); - return transaction.run() - .then(() => datastore.get(taskListKey)) - .then((results) => { - taskList = results[0]; - const query = datastore.createQuery('Task') - .hasAncestor(taskListKey); - return datastore.runQuery(query); + return transaction.run() + .then(() => transaction.get(taskKey)) + .then((results) => { + const task = results[0]; + if (task) { + // The task entity already exists. + return transaction.rollback(); + } else { + // Create the task entity. + transaction.save(taskEntity); + return transaction.commit(); + } + }) + .then(() => taskEntity) + .catch(() => transaction.rollback()); + } + // [END transactional_get_or_create] + + return getOrCreate(taskKey, {}) + .then((task) => { + assert(task, 'Should have a task.'); + return getOrCreate(taskKey, {}); }) - .then((results) => { - taskListEntities = results[0]; - return transaction.commit(); + .then((task) => { + assert(task, 'Should have a task.'); + // Restore `datastore` to the mock API. + datastore = datastoreMock; }) - .then(() => [taskList, taskListEntities]) - .catch(() => transaction.rollback()); + .catch((err) => { + // Restore `datastore` to the mock API. + datastore = datastoreMock; + return Promise.reject(err); + }); } - // [END transactional_single_entity_group_read_only] - return getTaskListEntities() - .then((results) => { - // Restore `datastore` to the mock API. - datastore = datastoreMock; - assert.equal(results.length, 2); - assert.equal(Array.isArray(results[1]), true); - }, (err) => { - // Restore `datastore` to the mock API. - datastore = datastoreMock; - return Promise.reject(err); - }); -}; + testSingleEntityGroupReadOnly () { + // Overwrite so the real Datastore instance is used in `transferFunds`. + const datastoreMock = datastore; + datastore = this.datastore; + + // [START transactional_single_entity_group_read_only] + function getTaskListEntities () { + let taskList, taskListEntities; + + const transaction = datastore.transaction(); + const taskListKey = datastore.key(['TaskList', 'default']); + + return transaction.run() + .then(() => datastore.get(taskListKey)) + .then((results) => { + taskList = results[0]; + const query = datastore.createQuery('Task') + .hasAncestor(taskListKey); + return datastore.runQuery(query); + }) + .then((results) => { + taskListEntities = results[0]; + return transaction.commit(); + }) + .then(() => [taskList, taskListEntities]) + .catch(() => transaction.rollback()); + } + // [END transactional_single_entity_group_read_only] + + return getTaskListEntities() + .then((results) => { + // Restore `datastore` to the mock API. + datastore = datastoreMock; + assert.equal(results.length, 2); + assert.equal(Array.isArray(results[1]), true); + }, (err) => { + // Restore `datastore` to the mock API. + datastore = datastoreMock; + return Promise.reject(err); + }); + } +} module.exports = { Entity: Entity, diff --git a/datastore/system-test/concepts.test.js b/datastore/system-test/concepts.test.js index 43281c0c71..2365dae13c 100644 --- a/datastore/system-test/concepts.test.js +++ b/datastore/system-test/concepts.test.js @@ -34,7 +34,7 @@ const Query = concepts.Query; describe(`datastore:concepts`, () => { before(() => { const projectId = process.env.GCLOUD_PROJECT; - assert(projectId, `You must set the GCLOUD_PROJECT env var!`); + assert.equal(!!projectId, true, `You must set the GCLOUD_PROJECT env var!`); transaction = new Transaction(projectId); metadata = new Metadata(projectId); index = new Index(projectId); diff --git a/datastore/tasks.js b/datastore/tasks.js index 40a35787dd..d60016a94e 100644 --- a/datastore/tasks.js +++ b/datastore/tasks.js @@ -154,20 +154,7 @@ function deleteTask (taskId) { } // [END delete_entity] -const cli = require(`yargs`); - -const program = module.exports = { - addTask, - markDone, - listTasks, - deleteTask, - main: (args) => { - // Run the command-line program - cli.help().strict().parse(args).argv; - } -}; - -cli +require(`yargs`) .demand(1) .command( `new `, @@ -199,8 +186,7 @@ cli .example(`node $0 delete 12345`, `Deletes task 12345.`) .wrap(120) .recommendCommands() - .epilogue(`For more information, see https://cloud.google.com/datastore/docs`); - -if (module === require.main) { - program.main(process.argv.slice(2)); -} + .epilogue(`For more information, see https://cloud.google.com/datastore/docs`) + .help() + .strict() + .argv;