From 497b7cb4449f257836c13e4da1deeb98b41bd393 Mon Sep 17 00:00:00 2001 From: Justin Beckwith Date: Thu, 16 Apr 2020 20:36:03 -0700 Subject: [PATCH 1/3] feat: add async getProjectId method --- package.json | 1 - src/index.ts | 16 +++++---------- src/request.ts | 53 ++++++++++++++++++++----------------------------- test/index.ts | 10 ---------- test/request.ts | 36 --------------------------------- 5 files changed, 27 insertions(+), 89 deletions(-) diff --git a/package.json b/package.json index 4dd8fdf80..a1755fdac 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,6 @@ "test": "c8 mocha build/test" }, "dependencies": { - "@google-cloud/projectify": "^2.0.0", "@google-cloud/promisify": "^2.0.0", "arrify": "^2.0.1", "concat-stream": "^2.0.0", diff --git a/src/index.ts b/src/index.ts index 53c50c197..fc3f1b976 100644 --- a/src/index.ts +++ b/src/index.ts @@ -381,7 +381,6 @@ const urlSafeKey = new entity.URLSafeKey(); class Datastore extends DatastoreRequest { clients_: Map; namespace?: string; - projectId: string; defaultBaseUrl_: string; options: DatastoreOptions; baseUrl_?: string; @@ -400,15 +399,7 @@ class Datastore extends DatastoreRequest { */ this.namespace = options.namespace; - const userProvidedProjectId = - options.projectId || process.env.DATASTORE_PROJECT_ID; - const defaultProjectId = '{{projectId}}'; - - /** - * @name Datastore#projectId - * @type {string} - */ - this.projectId = userProvidedProjectId || defaultProjectId; + options.projectId = options.projectId || process.env.DATASTORE_PROJECT_ID; this.defaultBaseUrl_ = 'datastore.googleapis.com'; this.determineBaseUrl_(options.apiEndpoint); @@ -420,7 +411,6 @@ class Datastore extends DatastoreRequest { scopes: gapic.v1.DatastoreClient.scopes, servicePath: this.baseUrl_, port: typeof this.port_ === 'number' ? this.port_ : 443, - projectId: userProvidedProjectId, }, options ); @@ -431,6 +421,10 @@ class Datastore extends DatastoreRequest { this.auth = new GoogleAuth(this.options); } + getProjectId(): Promise { + return this.auth.getProjectId(); + } + /** * Helper function to get a Datastore Double object. * diff --git a/src/request.ts b/src/request.ts index 39545fbfe..b284ec36c 100644 --- a/src/request.ts +++ b/src/request.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import {replaceProjectIdToken} from '@google-cloud/projectify'; import {promisifyAll} from '@google-cloud/promisify'; import arrify = require('arrify'); @@ -1346,9 +1345,7 @@ class DatastoreRequest { const isTransaction = this.id ? true : false; const method = config.method; - let reqOpts = extend(true, {}, config.reqOpts); - - reqOpts.projectId = datastore.projectId; + const reqOpts = extend(true, {}, config.reqOpts); // Set properties to indicate if we're in a transaction or not. if (method === 'commit') { @@ -1376,31 +1373,27 @@ class DatastoreRequest { }; } - datastore.auth.getProjectId( - (err: GetProjectIdErr, projectId: ProjectId) => { - if (err) { - callback!(err); - return; - } - - const clientName = config.client; - - if (!datastore.clients_.has(clientName)) { - datastore.clients_.set( - clientName, - new gapic.v1[clientName](datastore.options) - ); - } - const gaxClient = datastore.clients_.get(clientName); - reqOpts = replaceProjectIdToken(reqOpts, projectId!); - const gaxOpts = extend(true, {}, config.gaxOpts, { - headers: { - 'google-cloud-resource-prefix': `projects/${projectId}`, - }, - }); - gaxClient![method](reqOpts, gaxOpts, callback); + datastore.auth.getProjectId((err, projectId) => { + if (err) { + callback!(err); + return; } - ); + const clientName = config.client; + if (!datastore.clients_.has(clientName)) { + datastore.clients_.set( + clientName, + new gapic.v1[clientName](datastore.options) + ); + } + const gaxClient = datastore.clients_.get(clientName); + reqOpts.projectId = projectId!; + const gaxOpts = extend(true, {}, config.gaxOpts, { + headers: { + 'google-cloud-resource-prefix': `projects/${projectId}`, + }, + }); + gaxClient![method](reqOpts, gaxOpts, callback); + }); } } @@ -1438,7 +1431,6 @@ export interface GetCallback { (err?: Error | null, entity?: Entities): void; } export type GetResponse = [Entities]; -export type GetProjectIdErr = Error | null | undefined; export interface Mutation { [key: string]: EntityProto; } @@ -1450,7 +1442,6 @@ export interface PrepareEntityObjectResponse { data?: google.datastore.v1.Entity; method?: string; } -export type ProjectId = string | null | undefined; export interface RequestCallback { ( a?: Error | null, @@ -1476,7 +1467,7 @@ export interface RequestOptions { } | null; transaction?: string | null; mode?: string; - projectId?: ProjectId; + projectId?: string; query?: QueryProto; } export type RunQueryStreamOptions = RunQueryOptions; diff --git a/test/index.ts b/test/index.ts index 2d67ae988..a5a387623 100644 --- a/test/index.ts +++ b/test/index.ts @@ -190,15 +190,9 @@ describe('Datastore', () => { }); it('should localize the projectId', () => { - assert.strictEqual(datastore.projectId, PROJECT_ID); assert.strictEqual(datastore.options.projectId, PROJECT_ID); }); - it('should default project ID to placeholder', () => { - const datastore = new Datastore({}); - assert.strictEqual(datastore.projectId, '{{projectId}}'); - }); - it('should not default options.projectId to placeholder', () => { const datastore = new Datastore({}); assert.strictEqual(datastore.options.projectId, undefined); @@ -206,12 +200,8 @@ describe('Datastore', () => { it('should use DATASTORE_PROJECT_ID', () => { const projectId = 'overridden-project-id'; - process.env.DATASTORE_PROJECT_ID = projectId; - const datastore = new Datastore({}); - - assert.strictEqual(datastore.projectId, projectId); assert.strictEqual(datastore.options.projectId, projectId); }); diff --git a/test/request.ts b/test/request.ts index 91824f3d5..f3adf1978 100644 --- a/test/request.ts +++ b/test/request.ts @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -import * as pjy from '@google-cloud/projectify'; import * as pfy from '@google-cloud/promisify'; import * as assert from 'assert'; import {after, afterEach, before, beforeEach, describe, it} from 'mocha'; @@ -48,13 +47,6 @@ const fakePfy = Object.assign({}, pfy, { }, }); -const fakePjy = { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - replaceProjectIdToken(...args: any[]) { - return (pjyOverride || pjy.replaceProjectIdToken)(...args); - }, -}; - let v1FakeClientOverride: Function | null; const fakeV1 = { FakeClient: class { @@ -67,8 +59,6 @@ const fakeV1 = { class FakeQuery extends Query {} -let pjyOverride: Function | null; - describe('Request', () => { let Request: typeof ds.DatastoreRequest; let request: Any; @@ -78,7 +68,6 @@ describe('Request', () => { before(() => { Request = proxyquire('../src/request', { '@google-cloud/promisify': fakePfy, - '@google-cloud/projectify': fakePjy, './entity': {entity}, './query': {Query: FakeQuery}, './v1': fakeV1, @@ -90,7 +79,6 @@ describe('Request', () => { }); beforeEach(() => { - pjyOverride = null; key = new entity.Key({ namespace: 'namespace', path: ['Company', 123], @@ -2338,30 +2326,6 @@ describe('Request', () => { done(); }); - it('should replace the project ID token', done => { - const replacedReqOpts = {}; - - const expectedReqOpts: Any = Object.assign({}, CONFIG.reqOpts); - expectedReqOpts.projectId = request.projectId; - - pjyOverride = (reqOpts: RequestOptions, projectId: string) => { - assert.notStrictEqual(reqOpts, CONFIG.reqOpts); - assert.deepStrictEqual(reqOpts, expectedReqOpts); - assert.strictEqual(projectId, PROJECT_ID); - return replacedReqOpts; - }; - - request.datastore.clients_ = new Map(); - request.datastore.clients_.set(CONFIG.client, { - [CONFIG.method](reqOpts: RequestOptions) { - assert.strictEqual(reqOpts, replacedReqOpts); - done(); - }, - }); - - request.request_(CONFIG, assert.ifError); - }); - it('should send gaxOpts', done => { request.datastore.clients_ = new Map(); request.datastore.clients_.set(CONFIG.client, { From aa22c86a4462b28b8d6c2c3129c78be814656fba Mon Sep 17 00:00:00 2001 From: Justin Beckwith Date: Fri, 17 Apr 2020 07:19:53 -0700 Subject: [PATCH 2/3] remove bonus projectId --- src/transaction.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/transaction.ts b/src/transaction.ts index ea9002b4a..2331ce44f 100644 --- a/src/transaction.ts +++ b/src/transaction.ts @@ -49,7 +49,6 @@ import { * const transaction = datastore.transaction(); */ class Transaction extends DatastoreRequest { - projectId: string; namespace?: string; readOnly: boolean; request: Function; @@ -63,11 +62,6 @@ class Transaction extends DatastoreRequest { */ this.datastore = datastore; - /** - * @name Transaction#projectId - * @type {string} - */ - this.projectId = datastore.projectId; /** * @name Transaction#namespace * @type {string} From 4481870ebe7f5206b9fe0c2659ef898c12dc908c Mon Sep 17 00:00:00 2001 From: Justin Beckwith Date: Fri, 17 Apr 2020 07:26:32 -0700 Subject: [PATCH 3/3] untest --- test/transaction.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/transaction.ts b/test/transaction.ts index c1400bb00..29545d236 100644 --- a/test/transaction.ts +++ b/test/transaction.ts @@ -100,10 +100,6 @@ describe('Transaction', () => { assert.strictEqual(transaction.datastore, DATASTORE); }); - it('should localize the project ID', () => { - assert.strictEqual(transaction.projectId, PROJECT_ID); - }); - it('should localize the namespace', () => { assert.strictEqual(transaction.namespace, NAMESPACE); });