From c7c43ec25dd813abb6f9fee8a64248de438a6305 Mon Sep 17 00:00:00 2001 From: vijay-qlogic Date: Mon, 3 Dec 2018 17:08:50 +0530 Subject: [PATCH 1/8] removed async module dependency --- package.json | 1 - src/request.ts | 4 +- system-test/datastore.ts | 90 ++++++++++++++++------------------------ 3 files changed, 37 insertions(+), 58 deletions(-) diff --git a/package.json b/package.json index d426f859f..73c26bf4b 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,6 @@ "@types/proxyquire": "^1.3.28", "@types/sinon": "7.0.0", "@types/through2": "^2.0.34", - "async": "^2.6.1", "codecov": "^3.0.2", "eslint": "^5.0.0", "eslint-config-prettier": "^3.0.0", diff --git a/src/request.ts b/src/request.ts index 42a8a971c..9406db94b 100644 --- a/src/request.ts +++ b/src/request.ts @@ -332,7 +332,7 @@ class DatastoreRequest { * const apiResponse = data[0]; * }); */ - delete(keys, gaxOptions, callback?) { + delete(keys, gaxOptions?, callback?) { if (is.fn(gaxOptions)) { callback = gaxOptions; gaxOptions = {}; @@ -580,7 +580,7 @@ class DatastoreRequest { * const entities = data[0]; * }); */ - runQuery(query, options, callback?) { + runQuery(query, options?, callback?) { if (is.fn(options)) { callback = options; options = {}; diff --git a/system-test/datastore.ts b/system-test/datastore.ts index b54029533..f57e779f6 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -15,7 +15,6 @@ */ import * as assert from 'assert'; -import * as async from 'async'; import {Datastore} from '../src'; import {entity} from '../src/entity'; @@ -31,25 +30,24 @@ describe('Datastore', () => { return keyObject; }; - after(done => { - function deleteEntities(kind, callback) { - const query = datastore.createQuery(kind).select('__key__'); + after(async () => { + async function deleteEntities(kind) { + const query = await datastore.createQuery(kind).select('__key__'); - datastore.runQuery(query, (err, entities) => { - if (err) { - callback(err); - return; - } - - const keys = entities.map(entity => { - return entity[datastore.KEY]; - }); - - datastore.delete(keys, callback); + const data = await datastore.runQuery(query); + const entities = data[0]; + const keys = entities.map(entity => { + return entity[datastore.KEY]; }); + + await datastore.delete(keys); } - async.each(testKinds, deleteEntities, done); + let promises: Array<{}> = []; + for (let i = 0; i < testKinds.length; i++) { + promises.push(deleteEntities(testKinds[i])); + } + await Promise.all(promises); }); it('should allocate IDs', done => { @@ -816,50 +814,32 @@ describe('Datastore', () => { const transaction = datastore.transaction(); - transaction.run(err => { + transaction.commit(async err => { assert.ifError(err); - transaction.delete(deleteKey); - - transaction.save([ - { - key, - data: {rating: 10}, + // Incomplete key should have been given an ID. + assert.strictEqual(incompleteKey.path.length, 2); + + await Promise.all([ + // The key queued for deletion should have been deleted. + callback => { + datastore.get(deleteKey, (err, entity) => { + assert.ifError(err); + assert.strictEqual(typeof entity, 'undefined'); + callback(); + }); }, - { - key: incompleteKey, - data: {rating: 100}, + + // Data should have been updated on the key. + callback => { + datastore.get(key, (err, entity) => { + assert.ifError(err); + assert.strictEqual(entity.rating, 10); + callback(); + }); }, ]); - - transaction.commit(err => { - assert.ifError(err); - - // Incomplete key should have been given an ID. - assert.strictEqual(incompleteKey.path.length, 2); - - async.parallel( - [ - // The key queued for deletion should have been deleted. - callback => { - datastore.get(deleteKey, (err, entity) => { - assert.ifError(err); - assert.strictEqual(typeof entity, 'undefined'); - callback(); - }); - }, - - // Data should have been updated on the key. - callback => { - datastore.get(key, (err, entity) => { - assert.ifError(err); - assert.strictEqual(entity.rating, 10); - callback(); - }); - }, - ], - done); - }); + done(); }); }); }); From 0791a6801c837916f2c9ef124c4a20e3a7357c46 Mon Sep 17 00:00:00 2001 From: vijay-qlogic Date: Wed, 5 Dec 2018 23:15:46 +0530 Subject: [PATCH 2/8] updates to comments language --- src/request.ts | 12 ++++++------ system-test/datastore.ts | 8 ++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/request.ts b/src/request.ts index 9406db94b..f7fcdb2b5 100644 --- a/src/request.ts +++ b/src/request.ts @@ -161,7 +161,7 @@ class DatastoreRequest { * datastore.allocateIds(incompleteKey, 100, callback); * * //- - * // If the callback is omitted, we'll return a Promise. + * // Returns a Promise if callback is omitted. * //- * datastore.allocateIds(incompleteKey, 100).then((data) => { * const keys = data[0]; @@ -326,7 +326,7 @@ class DatastoreRequest { * ], (err, apiResponse) => {}); * * //- - * // If the callback is omitted, we'll return a Promise. + * // Returns a Promise if callback is omitted. * //- * datastore.delete().then((data) => { * const apiResponse = data[0]; @@ -425,7 +425,7 @@ class DatastoreRequest { * datastore.get(keys, (err, entities) => {}); * * //- - * // Here's how you would update the value of an entity with the help of the + * // Below is how to update the value of an entity with the help of the * // `save` method. * //- * datastore.get(key, (err, entity) => { @@ -442,7 +442,7 @@ class DatastoreRequest { * }); * * //- - * // If the callback is omitted, we'll return a Promise. + * // Returns a Promise if callback is omitted. * //- * datastore.get(keys).then((data) => { * const entities = data[0]; @@ -574,7 +574,7 @@ class DatastoreRequest { * }); * * //- - * // If the callback is omitted, we'll return a Promise. + * // Returns a Promise if callback is omitted. * //- * datastore.runQuery(query).then((data) => { * const entities = data[0]; @@ -922,7 +922,7 @@ class DatastoreRequest { * datastore.save(entity, (err, apiResponse) => {}); * * //- - * // If the callback is omitted, we'll return a Promise. + * // Returns a Promise if callback is omitted. * //- * datastore.save(entity).then((data) => { * const apiResponse = data[0]; diff --git a/system-test/datastore.ts b/system-test/datastore.ts index f57e779f6..50c1b6b83 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -22,7 +22,7 @@ describe('Datastore', () => { const testKinds: Array<{}> = []; const datastore = new Datastore(); // Override the Key method so we can track what keys are created during the - // tests. They are then deleted in the `after` hook. + // tests. Keys are then deleted in the `after` hook. const key = datastore.key; datastore.key = function(options) { const keyObject = key.call(this, options); @@ -43,7 +43,7 @@ describe('Datastore', () => { await datastore.delete(keys); } - let promises: Array<{}> = []; + const promises: Array<{}> = []; for (let i = 0; i < testKinds.length; i++) { promises.push(deleteEntities(testKinds[i])); } @@ -821,7 +821,7 @@ describe('Datastore', () => { assert.strictEqual(incompleteKey.path.length, 2); await Promise.all([ - // The key queued for deletion should have been deleted. + // Deletes the key that is in the deletion queue. callback => { datastore.get(deleteKey, (err, entity) => { assert.ifError(err); @@ -830,7 +830,7 @@ describe('Datastore', () => { }); }, - // Data should have been updated on the key. + // Updates data on the key. callback => { datastore.get(key, (err, entity) => { assert.ifError(err); From 96821014b3644448820288f1940f9eedf1f3551e Mon Sep 17 00:00:00 2001 From: vijay-qlogic Date: Thu, 6 Dec 2018 23:16:56 +0530 Subject: [PATCH 3/8] review updates --- src/request.ts | 2 +- src/transaction.ts | 4 +- system-test/datastore.ts | 83 ++++++++++++++++++++-------------------- 3 files changed, 44 insertions(+), 45 deletions(-) diff --git a/src/request.ts b/src/request.ts index f7fcdb2b5..7c691a4cb 100644 --- a/src/request.ts +++ b/src/request.ts @@ -928,7 +928,7 @@ class DatastoreRequest { * const apiResponse = data[0]; * }); */ - save(entities, gaxOptions, callback?) { + save(entities, gaxOptions?, callback?) { entities = arrify(entities); if (is.fn(gaxOptions)) { diff --git a/src/transaction.ts b/src/transaction.ts index 9dab1b610..79c58d422 100644 --- a/src/transaction.ts +++ b/src/transaction.ts @@ -125,7 +125,7 @@ class Transaction extends DatastoreRequest { * const apiResponse = data[0]; * }); */ - commit(gaxOptions, callback?) { + commit(gaxOptions?, callback?) { if (is.fn(gaxOptions)) { callback = gaxOptions; gaxOptions = {}; @@ -429,7 +429,7 @@ class Transaction extends DatastoreRequest { * const apiResponse = data[1]; * }); */ - run(options, callback?) { + run(options?, callback?) { if (is.fn(options)) { callback = options; options = {}; diff --git a/system-test/datastore.ts b/system-test/datastore.ts index 50c1b6b83..144871ff3 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -32,10 +32,10 @@ describe('Datastore', () => { after(async () => { async function deleteEntities(kind) { - const query = await datastore.createQuery(kind).select('__key__'); + const query = datastore.createQuery(kind).select('__key__'); - const data = await datastore.runQuery(query); - const entities = data[0]; + const [entities] = await datastore.runQuery(query); + // const entities = data[0]; const keys = entities.map(entity => { return entity[datastore.KEY]; }); @@ -43,10 +43,7 @@ describe('Datastore', () => { await datastore.delete(keys); } - const promises: Array<{}> = []; - for (let i = 0; i < testKinds.length; i++) { - promises.push(deleteEntities(testKinds[i])); - } + const promises = testKinds.map(kind => deleteEntities(kind)) await Promise.all(promises); }); @@ -799,49 +796,51 @@ describe('Datastore', () => { }); }); - it('should commit all saves and deletes at the end', done => { + it('should commit all saves and deletes at the end', async () => { const deleteKey = datastore.key(['Company', 'Subway']); const key = datastore.key(['Company', 'Google']); const incompleteKey = datastore.key('Company'); - datastore.save( - { - key: deleteKey, - data: {}, - }, - err => { - assert.ifError(err); + await datastore.save( + { + key: deleteKey, + data: {}, + } + ); + const transaction = datastore.transaction(); - const transaction = datastore.transaction(); + await transaction.run(); + transaction.delete(deleteKey); - transaction.commit(async err => { - assert.ifError(err); + transaction.save([ + { + key, + data: {rating: 10}, + }, + { + key: incompleteKey, + data: {rating: 100}, + }, + ]); - // Incomplete key should have been given an ID. - assert.strictEqual(incompleteKey.path.length, 2); - - await Promise.all([ - // Deletes the key that is in the deletion queue. - callback => { - datastore.get(deleteKey, (err, entity) => { - assert.ifError(err); - assert.strictEqual(typeof entity, 'undefined'); - callback(); - }); - }, + await transaction.commit(); - // Updates data on the key. - callback => { - datastore.get(key, (err, entity) => { - assert.ifError(err); - assert.strictEqual(entity.rating, 10); - callback(); - }); - }, - ]); - done(); - }); - }); + // Incomplete key should have been given an ID. + assert.strictEqual(incompleteKey.path.length, 2); + + await Promise.all([ + // Deletes the key that is in the deletion queue. + datastore.get(deleteKey, (err, entity) => { + assert.ifError(err); + assert.strictEqual(typeof entity, 'undefined'); + }), + + // Updates data on the key. + datastore.get(key, (err, entity) => { + assert.ifError(err); + assert.strictEqual(entity.rating, 10); + }) + ]); }); it('should use the last modification to a key', done => { From 6e06d9a744136aa4cdcd436184556e7a5b31c847 Mon Sep 17 00:00:00 2001 From: vijay-qlogic Date: Thu, 6 Dec 2018 23:37:57 +0530 Subject: [PATCH 4/8] comment removed --- system-test/datastore.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/system-test/datastore.ts b/system-test/datastore.ts index 144871ff3..ff3f9b0ba 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -35,7 +35,6 @@ describe('Datastore', () => { const query = datastore.createQuery(kind).select('__key__'); const [entities] = await datastore.runQuery(query); - // const entities = data[0]; const keys = entities.map(entity => { return entity[datastore.KEY]; }); From a4e18db3d88c311f7b00d9e85c44a9702858d753 Mon Sep 17 00:00:00 2001 From: vijay-qlogic Date: Fri, 7 Dec 2018 14:46:44 +0530 Subject: [PATCH 5/8] fix for failing tests --- src/request.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/request.ts b/src/request.ts index 7c691a4cb..44b8e3e8f 100644 --- a/src/request.ts +++ b/src/request.ts @@ -580,7 +580,7 @@ class DatastoreRequest { * const entities = data[0]; * }); */ - runQuery(query, options?, callback?) { + runQuery(query, options?, callback?): void|Promise { if (is.fn(options)) { callback = options; options = {}; From a16a3bea73ccc718753a41441b102590eb5fdb8c Mon Sep 17 00:00:00 2001 From: vijay-qlogic Date: Wed, 12 Dec 2018 00:17:27 +0530 Subject: [PATCH 6/8] code review edits --- src/request.ts | 3 ++- system-test/datastore.ts | 16 +++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/request.ts b/src/request.ts index 44b8e3e8f..0817d3b78 100644 --- a/src/request.ts +++ b/src/request.ts @@ -448,7 +448,8 @@ class DatastoreRequest { * const entities = data[0]; * }); */ - get(keys, options, callback?) { + get(keys, options?): Promise; + get(keys, options, callback?): void|Promise { if (is.fn(options)) { callback = options; options = {}; diff --git a/system-test/datastore.ts b/system-test/datastore.ts index ff3f9b0ba..afa621084 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -17,6 +17,7 @@ import * as assert from 'assert'; import {Datastore} from '../src'; import {entity} from '../src/entity'; +import {promisify} from '@google-cloud/promisify'; describe('Datastore', () => { const testKinds: Array<{}> = []; @@ -829,16 +830,21 @@ describe('Datastore', () => { await Promise.all([ // Deletes the key that is in the deletion queue. - datastore.get(deleteKey, (err, entity) => { - assert.ifError(err); + datastore.get(deleteKey) + .then(([entity]) => { assert.strictEqual(typeof entity, 'undefined'); + }) + .catch(err => { + assert.ifError(err); }), - // Updates data on the key. - datastore.get(key, (err, entity) => { - assert.ifError(err); + datastore.get(key) + .then(([entity]) => { assert.strictEqual(entity.rating, 10); }) + .catch(err => { + assert.ifError(err); + }) ]); }); From 1d374b95ff40df80d9eb1a8e9b8beb7d1738db81 Mon Sep 17 00:00:00 2001 From: vijay-qlogic Date: Fri, 14 Dec 2018 11:52:42 +0530 Subject: [PATCH 7/8] Added review updates --- system-test/datastore.ts | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/system-test/datastore.ts b/system-test/datastore.ts index afa621084..c563bfde0 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -17,7 +17,6 @@ import * as assert from 'assert'; import {Datastore} from '../src'; import {entity} from '../src/entity'; -import {promisify} from '@google-cloud/promisify'; describe('Datastore', () => { const testKinds: Array<{}> = []; @@ -43,8 +42,7 @@ describe('Datastore', () => { await datastore.delete(keys); } - const promises = testKinds.map(kind => deleteEntities(kind)) - await Promise.all(promises); + await Promise.all(testKinds.map(kind => deleteEntities(kind))); }); it('should allocate IDs', done => { @@ -828,24 +826,14 @@ describe('Datastore', () => { // Incomplete key should have been given an ID. assert.strictEqual(incompleteKey.path.length, 2); - await Promise.all([ + const [[deletedEntity], [fetchedEntity]] = await Promise.all([ // Deletes the key that is in the deletion queue. - datastore.get(deleteKey) - .then(([entity]) => { - assert.strictEqual(typeof entity, 'undefined'); - }) - .catch(err => { - assert.ifError(err); - }), + datastore.get(deleteKey), // Updates data on the key. datastore.get(key) - .then(([entity]) => { - assert.strictEqual(entity.rating, 10); - }) - .catch(err => { - assert.ifError(err); - }) ]); + assert.strictEqual(typeof deletedEntity, 'undefined'); + assert.strictEqual(fetchedEntity.rating, 10); }); it('should use the last modification to a key', done => { From a56dfbbd1a7334a976d1cf48efc0c610169c8217 Mon Sep 17 00:00:00 2001 From: vijay-qlogic Date: Tue, 18 Dec 2018 22:00:08 +0530 Subject: [PATCH 8/8] conflicts resolved --- src/request.ts | 13 +++++++++---- src/transaction.ts | 2 +- system-test/datastore.ts | 16 +++++++--------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/request.ts b/src/request.ts index 0817d3b78..fe4bf7ab7 100644 --- a/src/request.ts +++ b/src/request.ts @@ -34,6 +34,10 @@ import {entity} from './entity'; import {Query} from './query'; import {Datastore} from '.'; +export interface EntityDataObj { + [key: string]: string; +} + /** * A map of read consistency values to proto codes. * @@ -448,8 +452,8 @@ class DatastoreRequest { * const entities = data[0]; * }); */ - get(keys, options?): Promise; - get(keys, options, callback?): void|Promise { + get(keys, options?): Promise; + get(keys, options?, callback?): void|Promise { if (is.fn(options)) { callback = options; options = {}; @@ -581,7 +585,8 @@ class DatastoreRequest { * const entities = data[0]; * }); */ - runQuery(query, options?, callback?): void|Promise { + runQuery(query, options?): Promise>>; + runQuery(query, options?, callback?): void|Promise>> { if (is.fn(options)) { callback = options; options = {}; @@ -1037,7 +1042,7 @@ class DatastoreRequest { client: 'DatastoreClient', method: 'commit', reqOpts, - gaxOpts: gaxOptions, + gaxOpts: gaxOptions || {}, }, onCommit); } diff --git a/src/transaction.ts b/src/transaction.ts index 79c58d422..c38419c49 100644 --- a/src/transaction.ts +++ b/src/transaction.ts @@ -215,7 +215,7 @@ class Transaction extends DatastoreRequest { client: 'DatastoreClient', method: 'commit', reqOpts, - gaxOpts: gaxOptions, + gaxOpts: gaxOptions || {}, }, (err, resp) => { if (err) { diff --git a/system-test/datastore.ts b/system-test/datastore.ts index c563bfde0..5dd666a3e 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -22,7 +22,7 @@ describe('Datastore', () => { const testKinds: Array<{}> = []; const datastore = new Datastore(); // Override the Key method so we can track what keys are created during the - // tests. Keys are then deleted in the `after` hook. + // tests. They are then deleted in the `after` hook. const key = datastore.key; datastore.key = function(options) { const keyObject = key.call(this, options); @@ -280,7 +280,7 @@ describe('Datastore', () => { data: post, }, err => { - assert.notStrictEqual(err, null); // should fail insert + assert.notStrictEqual(err, null); // should fail insert. datastore.get(postKey, (err, entity) => { assert.ifError(err); assert.deepStrictEqual(entity, post); @@ -799,18 +799,16 @@ describe('Datastore', () => { const key = datastore.key(['Company', 'Google']); const incompleteKey = datastore.key('Company'); - await datastore.save( - { - key: deleteKey, - data: {}, - } - ); + await datastore.save({ + key: deleteKey, + data: {}, + }); const transaction = datastore.transaction(); await transaction.run(); transaction.delete(deleteKey); - transaction.save([ + await transaction.save([ { key, data: {rating: 10},