Skip to content

Commit

Permalink
fix: replace projectId placeholder in formatted names (#1407)
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 #1375
  • Loading branch information
@olavloite
olavloite committed Jun 30, 2021
1 parent 6496fe1 commit 4364d2b
Show file tree
Hide file tree
Showing 6 changed files with 199 additions and 18 deletions.
6 changes: 6 additions & 0 deletions README.md
View file
Expand Up @@ -108,7 +108,13 @@ Samples are in the [`samples/`](https://github.com/googleapis/nodejs-spanner/tre
| Datatypes | [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/datatypes.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/datatypes.js,samples/README.md) |
| DML | [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/dml.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/dml.js,samples/README.md) |
| Get-commit-stats | [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/get-commit-stats.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/get-commit-stats.js,samples/README.md) |
| Creates a new value-storing index | [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/index-create-storing.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/index-create-storing.js,samples/README.md) |
| Creates a new index | [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/index-create.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/index-create.js,samples/README.md) |
| Executes a read-only SQL query using an existing index. | [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/index-query-data.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/index-query-data.js,samples/README.md) |
| Reads data using an existing storing index. | [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/index-read-data-with-storing.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/index-read-data-with-storing.js,samples/README.md) |
| Read data using an existing index. | [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/index-read-data.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/index-read-data.js,samples/README.md) |
| Indexing | [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/indexing.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/indexing.js,samples/README.md) |
| Instance-with-processing-units | [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/instance-with-processing-units.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/instance-with-processing-units.js,samples/README.md) |
| Instance | [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/instance.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/instance.js,samples/README.md) |
| Numeric-add-column | [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/numeric-add-column.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/numeric-add-column.js,samples/README.md) |
| Numeric-query-parameter | [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/numeric-query-parameter.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/numeric-query-parameter.js,samples/README.md) |
Expand Down
108 changes: 108 additions & 0 deletions samples/README.md
View file
Expand Up @@ -32,7 +32,13 @@ and automatic, synchronous replication for high availability.
* [Datatypes](#datatypes)
* [DML](#dml)
* [Get-commit-stats](#get-commit-stats)
* [Creates a new value-storing index](#creates-a-new-value-storing-index)
* [Creates a new index](#creates-a-new-index)
* [Executes a read-only SQL query using an existing index.](#executes-a-read-only-sql-query-using-an-existing-index.)
* [Reads data using an existing storing index.](#reads-data-using-an-existing-storing-index.)
* [Read data using an existing index.](#read-data-using-an-existing-index.)
* [Indexing](#indexing)
* [Instance-with-processing-units](#instance-with-processing-units)
* [Instance](#instance)
* [Numeric-add-column](#numeric-add-column)
* [Numeric-query-parameter](#numeric-query-parameter)
Expand Down Expand Up @@ -366,6 +372,91 @@ __Usage:__



### Creates a new value-storing index

View the [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/index-create-storing.js).

[![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/index-create-storing.js,samples/README.md)

__Usage:__


`node createStoringIndex <INSTANCE_ID> <DATABASE_ID> <PROJECT_ID>`


-----




### Creates a new index

View the [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/index-create.js).

[![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/index-create.js,samples/README.md)

__Usage:__


`node createIndex <INSTANCE_ID> <DATABASE_ID> <PROJECT_ID>`


-----




### Executes a read-only SQL query using an existing index.

View the [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/index-query-data.js).

[![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/index-query-data.js,samples/README.md)

__Usage:__


`node queryDataWithIndex <INSTANCE_ID> <DATABASE_ID> <PROJECT_ID> <START_TITLE> <END_TITLE>`


-----




### Reads data using an existing storing index.

View the [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/index-read-data-with-storing.js).

[![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/index-read-data-with-storing.js,samples/README.md)

__Usage:__


`node readDataWithStoringIndex <INSTANCE_ID> <DATABASE_ID> <PROJECT_ID>`


-----




### Read data using an existing index.

View the [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/index-read-data.js).

[![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/index-read-data.js,samples/README.md)

__Usage:__


`node readDataWithIndex <INSTANCE_ID> <DATABASE_ID> <PROJECT_ID>`


-----




### Indexing

View the [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/indexing.js).
Expand All @@ -383,6 +474,23 @@ __Usage:__



### Instance-with-processing-units

View the [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/instance-with-processing-units.js).

[![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/instance-with-processing-units.js,samples/README.md)

__Usage:__


`node samples/instance-with-processing-units.js`


-----




### Instance

View the [source code](https://github.com/googleapis/nodejs-spanner/blob/master/samples/instance.js).
Expand Down
30 changes: 30 additions & 0 deletions src/index.ts
View file
Expand Up @@ -158,6 +158,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 @@ -260,6 +261,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 @@ -889,6 +891,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
Expand Up @@ -1400,8 +1400,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
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

0 comments on commit 4364d2b

Please sign in to comment.