feat: add async getProjectId method (#657)

BREAKING CHANGE: The `Datastore.projectId` property has been removed, and replaced with an asynchronous `getProjectid()` method. The projectId cannot be determined synchronously, so the previous approach was to use a `{{projectId}}` string placeholder if the projectId had not yet been acquired. This made it difficult to know exactly when the property would be defined. As an added bonus, the `@google-cloud/projectify` dependency has been removed. Adding @crwilcox because this would be a breaking change (we're about to ship one).
googleapis · Apr 20, 2020 · 2a7e034 · 2a7e034
1 parent ea22d60
commit 2a7e034
Show file tree

Hide file tree

Showing 7 changed files with 27 additions and 99 deletions.
diff --git 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
@@ -381,7 +381,6 @@ const urlSafeKey = new entity.URLSafeKey();
 class Datastore extends DatastoreRequest {
   clients_: Map<string, ClientStub>;
   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<string> {
+    return this.auth.getProjectId();
+  }
+
   /**
    * Helper function to get a Datastore Double object.
    *

diff --git 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/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}

diff --git a/test/index.ts b/test/index.ts
@@ -190,28 +190,18 @@ 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);
     });
 
     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
@@ -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, {

diff --git 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);
     });