Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add async getProjectId method #657

Merged
merged 4 commits into from
Apr 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
16 changes: 5 additions & 11 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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
);
Expand All @@ -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.
*
Expand Down
53 changes: 22 additions & 31 deletions src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
* limitations under the License.
*/

import {replaceProjectIdToken} from '@google-cloud/projectify';
import {promisifyAll} from '@google-cloud/promisify';
import arrify = require('arrify');

Expand Down Expand Up @@ -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') {
Expand Down Expand Up @@ -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);
});
}
}

Expand Down Expand Up @@ -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;
}
Expand All @@ -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,
Expand All @@ -1476,7 +1467,7 @@ export interface RequestOptions {
} | null;
transaction?: string | null;
mode?: string;
projectId?: ProjectId;
projectId?: string;
query?: QueryProto;
}
export type RunQueryStreamOptions = RunQueryOptions;
Expand Down
6 changes: 0 additions & 6 deletions src/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ import {
* const transaction = datastore.transaction();
*/
class Transaction extends DatastoreRequest {
projectId: string;
namespace?: string;
readOnly: boolean;
request: Function;
Expand All @@ -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}
Expand Down
10 changes: 0 additions & 10 deletions test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand Down
36 changes: 0 additions & 36 deletions test/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 {
Expand All @@ -67,8 +59,6 @@ const fakeV1 = {

class FakeQuery extends Query {}

let pjyOverride: Function | null;

describe('Request', () => {
let Request: typeof ds.DatastoreRequest;
let request: Any;
Expand All @@ -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,
Expand All @@ -90,7 +79,6 @@ describe('Request', () => {
});

beforeEach(() => {
pjyOverride = null;
key = new entity.Key({
namespace: 'namespace',
path: ['Company', 123],
Expand Down Expand Up @@ -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, {
Expand Down
4 changes: 0 additions & 4 deletions test/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down