Skip to content

Commit

Permalink
fix: replace projectId placeholder in formatted names
Browse files Browse the repository at this point in the history 
The {{projectId}} placeholder was only replaced in the names of the objects that
were included in the requests, and not in the headers and formatted names. This
would cause the CLOUD_RESOURCE_HEADER to be filled with the {{projectId}} placeholder
instead of the actual value.
As the actual project ID is gotten through an async method, and the constructor and
`instance` methods are synchronous, we can only replace the actual values once it is
retrieved for the first request.

Fixes googleapis#1375
  • Loading branch information
@olavloite
olavloite committed Jun 25, 2021
1 parent dff972b commit b982e2d
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 18 deletions.
30 changes: 30 additions & 0 deletions src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ class Spanner extends GrpcService {
auth: GoogleAuth;
clients_: Map<string, {}>;
instances_: Map<string, Instance>;
projectIdReplaced_: boolean;
projectFormattedName_: string;
resourceHeader_: {[k: string]: string};

Expand Down Expand Up @@ -259,6 +260,7 @@ class Spanner extends GrpcService {
this.auth = new GoogleAuth(this.options);
this.clients_ = new Map();
this.instances_ = new Map();
this.projectIdReplaced_ = false;
this.projectFormattedName_ = 'projects/' + this.projectId;
this.resourceHeader_ = {
[CLOUD_RESOURCE_HEADER]: this.projectFormattedName_,
Expand Down Expand Up @@ -876,6 +878,34 @@ class Spanner extends GrpcService {
const gaxClient = this.clients_.get(clientName)!;
let reqOpts = extend(true, {}, config.reqOpts);
reqOpts = replaceProjectIdToken(reqOpts, projectId!);
// It would have been preferable to replace the projectId already in the
// constructor of Spanner, but that is not possible as auth.getProjectId
// is an async method. This is therefore the first place where we have
// access to the value that should be used instead of the placeholder.
if (!this.projectIdReplaced_) {
this.projectId = replaceProjectIdToken(this.projectId, projectId!);
this.projectFormattedName_ = replaceProjectIdToken(
this.projectFormattedName_,
projectId!
);
this.instances_.forEach(instance => {
instance.formattedName_ = replaceProjectIdToken(
instance.formattedName_,
projectId!
);
instance.databases_.forEach(database => {
database.formattedName_ = replaceProjectIdToken(
database.formattedName_,
projectId!
);
});
});
this.projectIdReplaced_ = true;
}
config.headers[CLOUD_RESOURCE_HEADER] = replaceProjectIdToken(
config.headers[CLOUD_RESOURCE_HEADER],
projectId!
);
const requestFn = gaxClient[config.method].bind(
gaxClient,
reqOpts,
Expand Down
6 changes: 4 additions & 2 deletions test/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1373,8 +1373,10 @@ describe('Spanner', () => {
const replacedReqOpts = {};

replaceProjectIdTokenOverride = (reqOpts, projectId) => {
assert.deepStrictEqual(reqOpts, CONFIG.reqOpts);
assert.notStrictEqual(reqOpts, CONFIG.reqOpts);
if (typeof(reqOpts) === 'object') {
assert.deepStrictEqual(reqOpts, CONFIG.reqOpts);
assert.notStrictEqual(reqOpts, CONFIG.reqOpts);
}
assert.strictEqual(projectId, PROJECT_ID);
return replacedReqOpts;
};
Expand Down
46 changes: 31 additions & 15 deletions test/mockserver/mockspanner.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import * as path from 'path';
import {google} from '../../protos/protos';
import {grpc} from 'google-gax';
import * as protoLoader from '@grpc/proto-loader';
// eslint-disable-next-line node/no-extraneous-import
import {Metadata} from '@grpc/grpc-js';
import {Transaction} from '../../src';
import protobuf = google.spanner.v1;
import Timestamp = google.protobuf.Timestamp;
Expand Down Expand Up @@ -221,6 +223,7 @@ interface Request {}
*/
export class MockSpanner {
private requests: Request[] = [];
private metadata: Metadata[] = [];
private frozen = 0;
private sessionCounter = 0;
private sessions: Map<string, protobuf.Session> = new Map<
Expand Down Expand Up @@ -274,6 +277,7 @@ export class MockSpanner {

resetRequests(): void {
this.requests = [];
this.metadata = [];
}

/**
Expand All @@ -283,6 +287,13 @@ export class MockSpanner {
return this.requests;
}

/**
* @return the metadata that have been received by this mock server.
*/
getMetadata(): Metadata[] {
return this.metadata;
}

/**
* Registers a result for an SQL statement on the server.
* @param sql The SQL statement that should return the result.
Expand Down Expand Up @@ -417,14 +428,19 @@ export class MockSpanner {
return undefined;
}

private pushRequest(request: Request, metadata: Metadata): void {
this.requests.push(request);
this.metadata.push(metadata);
}

batchCreateSessions(
call: grpc.ServerUnaryCall<
protobuf.BatchCreateSessionsRequest,
protobuf.BatchCreateSessionsResponse
>,
callback: protobuf.Spanner.BatchCreateSessionsCallback
) {
this.requests.push(call.request!);
this.pushRequest(call.request!, call.metadata);
this.simulateExecutionTime(this.batchCreateSessions.name)
.then(() => {
const sessions = new Array<protobuf.Session>();
Expand All @@ -445,7 +461,7 @@ export class MockSpanner {
call: grpc.ServerUnaryCall<protobuf.CreateSessionRequest, protobuf.Session>,
callback: protobuf.Spanner.CreateSessionCallback
) {
this.requests.push(call.request!);
this.pushRequest(call.request!, call.metadata);
this.simulateExecutionTime(this.createSession.name).then(() => {
callback(null, this.newSession(call.request!.database));
});
Expand All @@ -455,7 +471,7 @@ export class MockSpanner {
call: grpc.ServerUnaryCall<protobuf.GetSessionRequest, protobuf.Session>,
callback: protobuf.Spanner.GetSessionCallback
) {
this.requests.push(call.request!);
this.pushRequest(call.request!, call.metadata);
this.simulateExecutionTime(this.getSession.name).then(() => {
const session = this.sessions[call.request!.name];
if (session) {
Expand All @@ -473,7 +489,7 @@ export class MockSpanner {
>,
callback: protobuf.Spanner.ListSessionsCallback
) {
this.requests.push(call.request!);
this.pushRequest(call.request!, call.metadata);
this.simulateExecutionTime(this.listSessions.name).then(() => {
callback(
null,
Expand All @@ -493,7 +509,7 @@ export class MockSpanner {
>,
callback: protobuf.Spanner.DeleteSessionCallback
) {
this.requests.push(call.request!);
this.pushRequest(call.request!, call.metadata);
if (this.sessions.delete(call.request!.name)) {
callback(null, google.protobuf.Empty.create());
} else {
Expand All @@ -505,7 +521,7 @@ export class MockSpanner {
call: grpc.ServerUnaryCall<protobuf.ExecuteSqlRequest, {}>,
callback: protobuf.Spanner.ExecuteSqlCallback
) {
this.requests.push(call.request!);
this.pushRequest(call.request!, call.metadata);
callback(createUnimplementedError('ExecuteSql is not yet implemented'));
}

Expand All @@ -515,7 +531,7 @@ export class MockSpanner {
protobuf.PartialResultSet
>
) {
this.requests.push(call.request!);
this.pushRequest(call.request!, call.metadata);
this.simulateExecutionTime(this.executeStreamingSql.name)
.then(() => {
if (call.request!.transaction && call.request!.transaction.id) {
Expand Down Expand Up @@ -687,7 +703,7 @@ export class MockSpanner {
>,
callback: protobuf.Spanner.ExecuteBatchDmlCallback
) {
this.requests.push(call.request!);
this.pushRequest(call.request!, call.metadata);
this.simulateExecutionTime(this.executeBatchDml.name)
.then(() => {
if (call.request!.transaction && call.request!.transaction.id) {
Expand Down Expand Up @@ -778,12 +794,12 @@ export class MockSpanner {
call: grpc.ServerUnaryCall<protobuf.ReadRequest, {}>,
callback: protobuf.Spanner.ReadCallback
) {
this.requests.push(call.request!);
this.pushRequest(call.request!, call.metadata);
callback(createUnimplementedError('Read is not yet implemented'));
}

streamingRead(call: grpc.ServerWritableStream<protobuf.ReadRequest, {}>) {
this.requests.push(call.request!);
this.pushRequest(call.request!, call.metadata);
call.emit(
'error',
createUnimplementedError('StreamingRead is not yet implemented')
Expand All @@ -798,7 +814,7 @@ export class MockSpanner {
>,
callback: protobuf.Spanner.BeginTransactionCallback
) {
this.requests.push(call.request!);
this.pushRequest(call.request!, call.metadata);
this.simulateExecutionTime(this.beginTransaction.name)
.then(() => {
const session = this.sessions.get(call.request!.session);
Expand Down Expand Up @@ -838,7 +854,7 @@ export class MockSpanner {
call: grpc.ServerUnaryCall<protobuf.CommitRequest, protobuf.CommitResponse>,
callback: protobuf.Spanner.CommitCallback
) {
this.requests.push(call.request!);
this.pushRequest(call.request!, call.metadata);
this.simulateExecutionTime(this.commit.name)
.then(() => {
if (call.request!.transactionId) {
Expand Down Expand Up @@ -888,7 +904,7 @@ export class MockSpanner {
call: grpc.ServerUnaryCall<protobuf.RollbackRequest, google.protobuf.Empty>,
callback: protobuf.Spanner.RollbackCallback
) {
this.requests.push(call.request!);
this.pushRequest(call.request!, call.metadata);
const session = this.sessions.get(call.request!.session);
if (session) {
const buffer = Buffer.from(call.request!.transactionId as string);
Expand All @@ -911,15 +927,15 @@ export class MockSpanner {
call: grpc.ServerUnaryCall<protobuf.PartitionQueryRequest, {}>,
callback: protobuf.Spanner.PartitionQueryCallback
) {
this.requests.push(call.request!);
this.pushRequest(call.request!, call.metadata);
callback(createUnimplementedError('PartitionQuery is not yet implemented'));
}

partitionRead(
call: grpc.ServerUnaryCall<protobuf.PartitionReadRequest, {}>,
callback: protobuf.Spanner.PartitionReadCallback
) {
this.requests.push(call.request!);
this.pushRequest(call.request!, call.metadata);
callback(createUnimplementedError('PartitionQuery is not yet implemented'));
}
}
Expand Down
40 changes: 39 additions & 1 deletion test/spanner.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import Priority = google.spanner.v1.RequestOptions.Priority;
import {PreciseDate} from '@google-cloud/precise-date';
import PartialResultSet = google.spanner.v1.PartialResultSet;
import protobuf = google.spanner.v1;
import {CLOUD_RESOURCE_HEADER} from '../src/common';

function numberToEnglishWord(num: number): string {
switch (num) {
Expand Down Expand Up @@ -135,8 +136,9 @@ describe('Spanner with mock server', () => {
// TODO(loite): Enable when SPANNER_EMULATOR_HOST is supported.
// Set environment variable for SPANNER_EMULATOR_HOST to the mock server.
// process.env.SPANNER_EMULATOR_HOST = `localhost:${port}`;
process.env.GOOGLE_CLOUD_PROJECT = 'test-project';
spanner = new Spanner({
projectId: 'fake-project-id',
// projectId: '{{projectId}}',
servicePath: 'localhost',
port,
sslCreds: grpc.credentials.createInsecure(),
Expand Down Expand Up @@ -188,6 +190,24 @@ describe('Spanner with mock server', () => {
}
});

it('should replace {{projectId}} in resource header', async () => {
const query = {
sql: selectSql,
};
const database = newTestDatabase();
try {
await database.run(query);
spannerMock.getMetadata().forEach(metadata => {
assert.strictEqual(
metadata.get(CLOUD_RESOURCE_HEADER)[0],
`projects/test-project/instances/instance/databases/${database.id}`
);
});
} finally {
await database.close();
}
});

it('should execute query with requestOptions', async () => {
const priority = RequestOptions.Priority.PRIORITY_HIGH;
const database = newTestDatabase();
Expand Down Expand Up @@ -3288,6 +3308,24 @@ describe('Spanner with mock server', () => {
);
});

it('should replace {{projectId}}', async () => {
const instance = spanner.instance(mockInstanceAdmin.PROD_INSTANCE_NAME);
const [updatedInstance] = await instance
.setMetadata({
nodeCount: 20,
displayName: 'Production instance with 20 nodes',
})
.then(data => {
return data[0].promise() as Promise<
[google.spanner.admin.instance.v1.Instance]
>;
})
.then(instance => {
return instance;
});
assert.strictEqual(updatedInstance.nodeCount, 20);
});

it('should update an instance', async () => {
const instance = spanner.instance(mockInstanceAdmin.PROD_INSTANCE_NAME);
const [updatedInstance] = await instance
Expand Down

0 comments on commit b982e2d

Please sign in to comment.