From 1b1947ab3b945ffcc0c8119888bdcbac4e52930d Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Thu, 11 Apr 2024 11:55:41 -0400 Subject: [PATCH 01/15] set up the test frame --- system-test/read-rows.ts | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/system-test/read-rows.ts b/system-test/read-rows.ts index 37beab74c..4b95591e5 100644 --- a/system-test/read-rows.ts +++ b/system-test/read-rows.ts @@ -12,19 +12,22 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {Bigtable} from '../src'; +import {Bigtable, Table} from '../src'; import {Mutation} from '../src/mutation.js'; const {tests} = require('../../system-test/data/read-rows-retry-test.json') as { tests: Test[]; }; import {google} from '../protos/protos'; import * as assert from 'assert'; -import {describe, it, afterEach, beforeEach} from 'mocha'; +import {describe, it, afterEach, beforeEach, before} from 'mocha'; import * as sinon from 'sinon'; import {EventEmitter} from 'events'; import {Test} from './testTypes'; import {ServiceError, GrpcClient, GoogleError, CallOptions} from 'google-gax'; import {PassThrough} from 'stream'; +import {MockServer} from '../src/util/mock-servers/mock-server'; +import {MockService} from '../src/util/mock-servers/mock-service'; +import {BigtableClientMockService} from '../src/util/mock-servers/service-implementations/bigtable-client-mock-service'; const {grpc} = new GrpcClient(); @@ -217,4 +220,28 @@ describe('Bigtable/Table', () => { }); }); }); + describe('createReadStream using mock server', () => { + let server: MockServer; + let service: MockService; + let bigtable = new Bigtable(); + let table: Table; + + before(async () => { + // make sure we have everything initialized before starting tests + const port = await new Promise(resolve => { + server = new MockServer(resolve); + }); + bigtable = new Bigtable({ + apiEndpoint: `localhost:${port}`, + }); + table = bigtable.instance('fake-instance').table('fake-table'); + service = new BigtableClientMockService(server); + }); + + tests.forEach(test => { + it(test.name, () => { + + }); + }); + }); }); From 59b2c26bf2e2c4a07e84d0f21952e416615d132f Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Thu, 11 Apr 2024 16:27:14 -0400 Subject: [PATCH 02/15] Test is set up to evaluate streaming behavior --- src/util/mock-servers/mock-server.ts | 1 - system-test/read-rows.ts | 113 ++++++++++++++++++++------- 2 files changed, 84 insertions(+), 30 deletions(-) diff --git a/src/util/mock-servers/mock-server.ts b/src/util/mock-servers/mock-server.ts index c4ebb9779..02ca9b7e4 100644 --- a/src/util/mock-servers/mock-server.ts +++ b/src/util/mock-servers/mock-server.ts @@ -37,7 +37,6 @@ export class MockServer { `localhost:${this.port}`, grpc.ServerCredentials.createInsecure(), () => { - server.start(); callback ? callback(portString) : undefined; } ); diff --git a/system-test/read-rows.ts b/system-test/read-rows.ts index 4b95591e5..10d9a4be8 100644 --- a/system-test/read-rows.ts +++ b/system-test/read-rows.ts @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {Bigtable, Table} from '../src'; +import {Bigtable, protos, Table} from '../src'; import {Mutation} from '../src/mutation.js'; const {tests} = require('../../system-test/data/read-rows-retry-test.json') as { tests: Test[]; @@ -28,6 +28,7 @@ import {PassThrough} from 'stream'; import {MockServer} from '../src/util/mock-servers/mock-server'; import {MockService} from '../src/util/mock-servers/mock-service'; import {BigtableClientMockService} from '../src/util/mock-servers/service-implementations/bigtable-client-mock-service'; +import {ServerWritableStream} from '@grpc/grpc-js'; const {grpc} = new GrpcClient(); @@ -78,6 +79,33 @@ function rowResponse(rowKey: {}) { }; } +function getRequestOptions(request: any): google.bigtable.v2.IRowSet { + const requestOptions = {} as google.bigtable.v2.IRowSet; + if (request.rows && request.rows.rowRanges) { + requestOptions.rowRanges = request.rows.rowRanges.map( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (range: any) => { + const convertedRowRange = {} as {[index: string]: string}; + Object.keys(range).forEach( + key => (convertedRowRange[key] = range[key].asciiSlice()) + ); + return convertedRowRange; + } + ); + } + if (request.rows && request.rows.rowKeys) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + requestOptions.rowKeys = request.rows.rowKeys.map((rowKeys: any) => + rowKeys.asciiSlice() + ); + } + if (request.rowsLimit) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (requestOptions as any).rowsLimit = request.rowsLimit; + } + return requestOptions; +} + describe('Bigtable/Table', () => { const bigtable = new Bigtable(); const INSTANCE_NAME = 'fake-instance2'; @@ -155,30 +183,7 @@ describe('Bigtable/Table', () => { requestedOptions = []; stub = sinon.stub(bigtable, 'request').callsFake(cfg => { const reqOpts = cfg.reqOpts; - const requestOptions = {} as google.bigtable.v2.IRowSet; - if (reqOpts.rows && reqOpts.rows.rowRanges) { - requestOptions.rowRanges = reqOpts.rows.rowRanges.map( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (range: any) => { - const convertedRowRange = {} as {[index: string]: string}; - Object.keys(range).forEach( - key => (convertedRowRange[key] = range[key].asciiSlice()) - ); - return convertedRowRange; - } - ); - } - if (reqOpts.rows && reqOpts.rows.rowKeys) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - requestOptions.rowKeys = reqOpts.rows.rowKeys.map((rowKeys: any) => - rowKeys.asciiSlice() - ); - } - if (reqOpts.rowsLimit) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (requestOptions as any).rowsLimit = reqOpts.rowsLimit; - } - requestedOptions.push(requestOptions); + requestedOptions.push(getRequestOptions(reqOpts)); rowKeysRead.push([]); const requestStream = new PassThrough({objectMode: true}); // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -220,13 +225,13 @@ describe('Bigtable/Table', () => { }); }); }); - describe('createReadStream using mock server', () => { + describe.only('createReadStream using mock server', () => { let server: MockServer; let service: MockService; let bigtable = new Bigtable(); let table: Table; - before(async () => { + beforeEach(async () => { // make sure we have everything initialized before starting tests const port = await new Promise(resolve => { server = new MockServer(resolve); @@ -239,8 +244,58 @@ describe('Bigtable/Table', () => { }); tests.forEach(test => { - it(test.name, () => { - + it(test.name, done => { + const requestedOptions: google.bigtable.v2.IRowSet[] = []; + // TODO: Replace any[] + const responses: any[] = test.responses as any[]; + const rowKeysRead: any[] = []; + let endCalled = false; + let error: ServiceError | null = null; + table.maxRetries = test.max_retries; + service.setService({ + ReadRows: ( + stream: ServerWritableStream< + protos.google.bigtable.v2.IReadRowsRequest, + protos.google.bigtable.v2.IReadRowsResponse + > + ) => { + const response = responses!.shift(); + assert(response); + requestedOptions.push(getRequestOptions(stream.request)); + stream.write({chunks: response.row_keys.map(rowResponse)}); + if (response.end_with_error) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const error: any = new Error(); + error.code = response.end_with_error; + stream.emit('error', error); + } else { + stream.end(); + } + }, + }); + table + .createReadStream(test.createReadStream_options) + .on('data', row => rowKeysRead[rowKeysRead.length - 1].push(row.id)) + .on('end', () => { + // TODO: Fix later + endCalled = true; + if (test.error) { + assert(!endCalled, ".on('end') should not have been invoked"); + assert.strictEqual(error!.code, test.error); + } else { + assert(endCalled, ".on('end') should have been invoked"); + assert.ifError(error); + } + assert.deepStrictEqual(rowKeysRead, test.row_keys_read); + assert.strictEqual( + responses.length, + 0, + 'not all the responses were used' + ); + assert.deepStrictEqual(requestedOptions, test.request_options); + done(); + }) + .on('error', err => (error = err as ServiceError)); }); }); }); From baa5d8f5d24a90f33b4b1b243ed47e363705f9a1 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Fri, 12 Apr 2024 10:17:04 -0400 Subject: [PATCH 03/15] Modify test with the mock server to pass first tst --- system-test/read-rows.ts | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/system-test/read-rows.ts b/system-test/read-rows.ts index 10d9a4be8..d16fd6683 100644 --- a/system-test/read-rows.ts +++ b/system-test/read-rows.ts @@ -79,6 +79,16 @@ function rowResponse(rowKey: {}) { }; } +function rowResponseFromServer(rowKey: string) { + return { + rowKey: Buffer.from(rowKey).toString('base64'), + familyName: {value: 'family'}, + qualifier: {value: Buffer.from('qualifier').toString('base64')}, + commitRow: true, + value: Buffer.from(rowKey).toString('base64'), + }; +} + function getRequestOptions(request: any): google.bigtable.v2.IRowSet { const requestOptions = {} as google.bigtable.v2.IRowSet; if (request.rows && request.rows.rowRanges) { @@ -99,7 +109,13 @@ function getRequestOptions(request: any): google.bigtable.v2.IRowSet { rowKeys.asciiSlice() ); } - if (request.rowsLimit) { + // The grpc protocol sets rowsLimit to '0' if rowsLimit is not provided in the + // grpc request. + // + // Do not append rowsLimit to collection of request options if received grpc + // rows limit is '0' so that test data in read-rows-retry-test.json remains + // shorter. + if (request.rowsLimit && request.rowsLimit !== '0') { // eslint-disable-next-line @typescript-eslint/no-explicit-any (requestOptions as any).rowsLimit = request.rowsLimit; } @@ -259,10 +275,14 @@ describe('Bigtable/Table', () => { protos.google.bigtable.v2.IReadRowsResponse > ) => { + console.log('entering readrows'); const response = responses!.shift(); assert(response); + rowKeysRead.push([]); requestedOptions.push(getRequestOptions(stream.request)); - stream.write({chunks: response.row_keys.map(rowResponse)}); + stream.write({ + chunks: response.row_keys.map(rowResponseFromServer), + }); if (response.end_with_error) { // eslint-disable-next-line @typescript-eslint/no-explicit-any const error: any = new Error(); @@ -295,7 +315,11 @@ describe('Bigtable/Table', () => { assert.deepStrictEqual(requestedOptions, test.request_options); done(); }) - .on('error', err => (error = err as ServiceError)); + .on('error', err => { + console.log('test'); + error = err as ServiceError; + throw err; + }); }); }); }); From 869d5ce4a43eada92ee4f3c53ada896a276b4dd3 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Fri, 12 Apr 2024 11:48:59 -0400 Subject: [PATCH 04/15] Fix tests that are appending startKey and endKey --- system-test/read-rows.ts | 57 ++++++++++++++++++++++++++-------------- 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/system-test/read-rows.ts b/system-test/read-rows.ts index d16fd6683..996c10eb4 100644 --- a/system-test/read-rows.ts +++ b/system-test/read-rows.ts @@ -96,6 +96,17 @@ function getRequestOptions(request: any): google.bigtable.v2.IRowSet { // eslint-disable-next-line @typescript-eslint/no-explicit-any (range: any) => { const convertedRowRange = {} as {[index: string]: string}; + { + // startKey and endKey get filled in during the grpc request. + // They should be removed as the test data does not look + // for these properties in the request. + if (range.startKey) { + delete range.startKey; + } + if (range.endKey) { + delete range.endKey; + } + } Object.keys(range).forEach( key => (convertedRowRange[key] = range[key].asciiSlice()) ); @@ -261,12 +272,32 @@ describe('Bigtable/Table', () => { tests.forEach(test => { it(test.name, done => { + // These variables store request/response data capturing data sent + // and received when using readRows with retries. This data is evaluated + // in checkResults at the end of the test for correctness. const requestedOptions: google.bigtable.v2.IRowSet[] = []; - // TODO: Replace any[] const responses: any[] = test.responses as any[]; const rowKeysRead: any[] = []; let endCalled = false; let error: ServiceError | null = null; + function checkResults() { + if (test.error) { + assert(!endCalled, ".on('end') should not have been invoked"); + assert.strictEqual(error!.code, test.error); + } else { + assert(endCalled, ".on('end') should have been invoked"); + assert.ifError(error); + } + assert.deepStrictEqual(rowKeysRead, test.row_keys_read); + assert.strictEqual( + responses.length, + 0, + 'not all the responses were used' + ); + assert.deepStrictEqual(requestedOptions, test.request_options); + done(); + } + table.maxRetries = test.max_retries; service.setService({ ReadRows: ( @@ -297,29 +328,15 @@ describe('Bigtable/Table', () => { .createReadStream(test.createReadStream_options) .on('data', row => rowKeysRead[rowKeysRead.length - 1].push(row.id)) .on('end', () => { - // TODO: Fix later endCalled = true; - if (test.error) { - assert(!endCalled, ".on('end') should not have been invoked"); - assert.strictEqual(error!.code, test.error); - } else { - assert(endCalled, ".on('end') should have been invoked"); - assert.ifError(error); - } - assert.deepStrictEqual(rowKeysRead, test.row_keys_read); - assert.strictEqual( - responses.length, - 0, - 'not all the responses were used' - ); - assert.deepStrictEqual(requestedOptions, test.request_options); - done(); - }) + checkResults(); + }); + /* .on('error', err => { - console.log('test'); error = err as ServiceError; - throw err; + checkResults(); }); + */ }); }); }); From 5e698340b09ac229a27828859077e56f2ee467af Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Fri, 12 Apr 2024 14:43:25 -0400 Subject: [PATCH 05/15] Getting all the tests working MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Transform the rowsLimit parameter into an integer - Change the hook into a before hook so that we don’t attempt to create multiple mock servers - Create a guard so that the stream only writes if there are row keys to write --- system-test/read-rows.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/system-test/read-rows.ts b/system-test/read-rows.ts index 996c10eb4..4f25c5dcc 100644 --- a/system-test/read-rows.ts +++ b/system-test/read-rows.ts @@ -128,7 +128,7 @@ function getRequestOptions(request: any): google.bigtable.v2.IRowSet { // shorter. if (request.rowsLimit && request.rowsLimit !== '0') { // eslint-disable-next-line @typescript-eslint/no-explicit-any - (requestOptions as any).rowsLimit = request.rowsLimit; + (requestOptions as any).rowsLimit = parseInt(request.rowsLimit); } return requestOptions; } @@ -258,7 +258,7 @@ describe('Bigtable/Table', () => { let bigtable = new Bigtable(); let table: Table; - beforeEach(async () => { + before(async () => { // make sure we have everything initialized before starting tests const port = await new Promise(resolve => { server = new MockServer(resolve); @@ -306,14 +306,15 @@ describe('Bigtable/Table', () => { protos.google.bigtable.v2.IReadRowsResponse > ) => { - console.log('entering readrows'); const response = responses!.shift(); assert(response); rowKeysRead.push([]); requestedOptions.push(getRequestOptions(stream.request)); - stream.write({ - chunks: response.row_keys.map(rowResponseFromServer), - }); + if (response.row_keys) { + stream.write({ + chunks: response.row_keys.map(rowResponseFromServer), + }); + } if (response.end_with_error) { // eslint-disable-next-line @typescript-eslint/no-explicit-any const error: any = new Error(); @@ -330,13 +331,11 @@ describe('Bigtable/Table', () => { .on('end', () => { endCalled = true; checkResults(); - }); - /* + }) .on('error', err => { error = err as ServiceError; checkResults(); }); - */ }); }); }); From bf29061fd5daee4f5be94090a322e08bee096710 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Fri, 12 Apr 2024 15:27:58 -0400 Subject: [PATCH 06/15] eliminate old createreadstream test define new interfaces too --- system-test/read-rows.ts | 133 ++------------------------------------- system-test/testTypes.ts | 21 +++++++ 2 files changed, 26 insertions(+), 128 deletions(-) diff --git a/system-test/read-rows.ts b/system-test/read-rows.ts index 4f25c5dcc..b585e99e0 100644 --- a/system-test/read-rows.ts +++ b/system-test/read-rows.ts @@ -13,18 +13,14 @@ // limitations under the License. import {Bigtable, protos, Table} from '../src'; -import {Mutation} from '../src/mutation.js'; const {tests} = require('../../system-test/data/read-rows-retry-test.json') as { - tests: Test[]; + tests: ReadRowsTest[]; }; import {google} from '../protos/protos'; import * as assert from 'assert'; -import {describe, it, afterEach, beforeEach, before} from 'mocha'; -import * as sinon from 'sinon'; -import {EventEmitter} from 'events'; -import {Test} from './testTypes'; -import {ServiceError, GrpcClient, GoogleError, CallOptions} from 'google-gax'; -import {PassThrough} from 'stream'; +import {describe, it, before} from 'mocha'; +import {ReadRowsTest} from './testTypes'; +import {ServiceError, GrpcClient, CallOptions} from 'google-gax'; import {MockServer} from '../src/util/mock-servers/mock-server'; import {MockService} from '../src/util/mock-servers/mock-service'; import {BigtableClientMockService} from '../src/util/mock-servers/service-implementations/bigtable-client-mock-service'; @@ -32,53 +28,6 @@ import {ServerWritableStream} from '@grpc/grpc-js'; const {grpc} = new GrpcClient(); -// eslint-disable-next-line @typescript-eslint/no-explicit-any -function dispatch(emitter: EventEmitter, response: any) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const emits: any[] = [{name: 'request'}]; - if (response.row_keys) { - emits.push.apply(emits, [ - {name: 'response', arg: 200}, - { - name: 'data', - arg: {chunks: response.row_keys.map(rowResponse)}, - }, - ]); - } - if (response.end_with_error) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const error: any = new Error(); - error.code = response.end_with_error; - emits.push({name: 'error', arg: error}); - } else { - emits.push({name: 'end'}); - } - let index = 0; - setImmediate(next); - - function next() { - if (index < emits.length) { - const emit = emits[index]; - index++; - emitter.emit(emit.name, emit.arg); - setImmediate(next); - } - } -} - -function rowResponse(rowKey: {}) { - return { - rowKey: Mutation.convertToBytes(rowKey), - familyName: {value: 'family'}, - qualifier: {value: 'qualifier'}, - valueSize: 0, - timestampMicros: 0, - labels: [], - commitRow: true, - value: 'value', - }; -} - function rowResponseFromServer(rowKey: string) { return { rowKey: Buffer.from(rowKey).toString('base64'), @@ -181,83 +130,11 @@ describe('Bigtable/Table', () => { }); }); - describe('createReadStream', () => { - let clock: sinon.SinonFakeTimers; - let endCalled: boolean; - let error: ServiceError | null; - let requestedOptions: Array<{}>; - let responses: Array<{}> | null; - let rowKeysRead: Array>; - let stub: sinon.SinonStub; - - beforeEach(() => { - clock = sinon.useFakeTimers({ - toFake: [ - 'setTimeout', - 'clearTimeout', - 'setImmediate', - 'clearImmediate', - 'setInterval', - 'clearInterval', - 'Date', - 'nextTick', - ], - }); - endCalled = false; - error = null; - responses = null; - rowKeysRead = []; - requestedOptions = []; - stub = sinon.stub(bigtable, 'request').callsFake(cfg => { - const reqOpts = cfg.reqOpts; - requestedOptions.push(getRequestOptions(reqOpts)); - rowKeysRead.push([]); - const requestStream = new PassThrough({objectMode: true}); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (requestStream as any).abort = () => {}; - dispatch(requestStream, responses!.shift()); - return requestStream; - }); - }); - - afterEach(() => { - clock.restore(); - stub.restore(); - }); - - tests.forEach(test => { - it(test.name, () => { - responses = test.responses; - TABLE.maxRetries = test.max_retries; - TABLE.createReadStream(test.createReadStream_options) - .on('data', row => rowKeysRead[rowKeysRead.length - 1].push(row.id)) - .on('end', () => (endCalled = true)) - .on('error', err => (error = err as ServiceError)); - clock.runAll(); - - if (test.error) { - assert(!endCalled, ".on('end') should not have been invoked"); - assert.strictEqual(error!.code, test.error); - } else { - assert(endCalled, ".on('end') shoud have been invoked"); - assert.ifError(error); - } - assert.deepStrictEqual(rowKeysRead, test.row_keys_read); - assert.strictEqual( - responses.length, - 0, - 'not all the responses were used' - ); - assert.deepStrictEqual(requestedOptions, test.request_options); - }); - }); - }); describe.only('createReadStream using mock server', () => { let server: MockServer; let service: MockService; let bigtable = new Bigtable(); let table: Table; - before(async () => { // make sure we have everything initialized before starting tests const port = await new Promise(resolve => { @@ -276,7 +153,7 @@ describe('Bigtable/Table', () => { // and received when using readRows with retries. This data is evaluated // in checkResults at the end of the test for correctness. const requestedOptions: google.bigtable.v2.IRowSet[] = []; - const responses: any[] = test.responses as any[]; + const responses = test.responses; const rowKeysRead: any[] = []; let endCalled = false; let error: ServiceError | null = null; diff --git a/system-test/testTypes.ts b/system-test/testTypes.ts index 43613cc67..f1b002341 100644 --- a/system-test/testTypes.ts +++ b/system-test/testTypes.ts @@ -14,6 +14,7 @@ import {ServiceError} from 'google-gax'; import {GetRowsOptions} from '../src/table'; +import {google} from '../protos/protos'; export interface Test { name: string; @@ -46,3 +47,23 @@ export interface Test { row_keys_read: {}; createReadStream_options: GetRowsOptions; } + +interface CreateReadStreamResponse { + row_keys?: string[]; + end_with_error?: 4; +} + +interface CreateReadStreamRequest { + rowKeys: string[]; + rowRanges: google.bigtable.v2.IRowRange[]; + rowsLimit: number; +} +export interface ReadRowsTest { + createReadStream_options?: GetRowsOptions; + max_retries: number; + responses: CreateReadStreamResponse[]; + request_options: CreateReadStreamRequest[]; + error: number; + row_keys_read: string[][]; + name: string; +} From ad36097976febfe31c1e8f5668ca90cf1cd253d7 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Fri, 12 Apr 2024 15:35:49 -0400 Subject: [PATCH 07/15] Remove only. rowsLimit should be optional --- system-test/read-rows.ts | 2 +- system-test/testTypes.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/system-test/read-rows.ts b/system-test/read-rows.ts index b585e99e0..851ba234a 100644 --- a/system-test/read-rows.ts +++ b/system-test/read-rows.ts @@ -130,7 +130,7 @@ describe('Bigtable/Table', () => { }); }); - describe.only('createReadStream using mock server', () => { + describe('createReadStream using mock server', () => { let server: MockServer; let service: MockService; let bigtable = new Bigtable(); diff --git a/system-test/testTypes.ts b/system-test/testTypes.ts index f1b002341..50d785c5d 100644 --- a/system-test/testTypes.ts +++ b/system-test/testTypes.ts @@ -56,7 +56,7 @@ interface CreateReadStreamResponse { interface CreateReadStreamRequest { rowKeys: string[]; rowRanges: google.bigtable.v2.IRowRange[]; - rowsLimit: number; + rowsLimit?: number; } export interface ReadRowsTest { createReadStream_options?: GetRowsOptions; From 2c57e6152d0929e11483ba56c4164cf518030dfe Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Fri, 12 Apr 2024 16:14:39 -0400 Subject: [PATCH 08/15] Make rowKeysRead type more specific --- system-test/read-rows.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system-test/read-rows.ts b/system-test/read-rows.ts index 851ba234a..8ad975e0d 100644 --- a/system-test/read-rows.ts +++ b/system-test/read-rows.ts @@ -154,7 +154,7 @@ describe('Bigtable/Table', () => { // in checkResults at the end of the test for correctness. const requestedOptions: google.bigtable.v2.IRowSet[] = []; const responses = test.responses; - const rowKeysRead: any[] = []; + const rowKeysRead: string[][] = []; let endCalled = false; let error: ServiceError | null = null; function checkResults() { From 95b9463c8691276d63b9288b5d1ee063f65414de Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Tue, 16 Apr 2024 09:44:23 -0400 Subject: [PATCH 09/15] Add after hook to shut down server --- system-test/read-rows.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/system-test/read-rows.ts b/system-test/read-rows.ts index 8ad975e0d..c2f5f4ea0 100644 --- a/system-test/read-rows.ts +++ b/system-test/read-rows.ts @@ -147,6 +147,10 @@ describe('Bigtable/Table', () => { service = new BigtableClientMockService(server); }); + after(async () => { + server.shutdown(() => {}); + }); + tests.forEach(test => { it(test.name, done => { // These variables store request/response data capturing data sent From 23919c56e88f147629a06c7b8cd627fd217eecaf Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Tue, 23 Apr 2024 15:41:48 -0400 Subject: [PATCH 10/15] First PR correction - remove any --- system-test/read-rows.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/system-test/read-rows.ts b/system-test/read-rows.ts index c2f5f4ea0..ddd8d5521 100644 --- a/system-test/read-rows.ts +++ b/system-test/read-rows.ts @@ -43,7 +43,7 @@ function getRequestOptions(request: any): google.bigtable.v2.IRowSet { if (request.rows && request.rows.rowRanges) { requestOptions.rowRanges = request.rows.rowRanges.map( // eslint-disable-next-line @typescript-eslint/no-explicit-any - (range: any) => { + (range: google.bigtable.v2.RowRange) => { const convertedRowRange = {} as {[index: string]: string}; { // startKey and endKey get filled in during the grpc request. @@ -56,8 +56,8 @@ function getRequestOptions(request: any): google.bigtable.v2.IRowSet { delete range.endKey; } } - Object.keys(range).forEach( - key => (convertedRowRange[key] = range[key].asciiSlice()) + Object.entries(range).forEach( + ([key, value]) => (convertedRowRange[key] = value.asciiSlice()) ); return convertedRowRange; } From abcf0fce2f0199f8adea3476e79b4d25fdb0b531 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Tue, 23 Apr 2024 16:02:04 -0400 Subject: [PATCH 11/15] Get rid of another any --- system-test/read-rows.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/system-test/read-rows.ts b/system-test/read-rows.ts index ddd8d5521..482ae7a85 100644 --- a/system-test/read-rows.ts +++ b/system-test/read-rows.ts @@ -38,7 +38,13 @@ function rowResponseFromServer(rowKey: string) { }; } -function getRequestOptions(request: any): google.bigtable.v2.IRowSet { +function getRequestOptions(request: { + rows: { + rowRanges: google.bigtable.v2.RowRange[]; + rowKeys: Uint8Array[]; + }; + rowsLimit: string; +}): google.bigtable.v2.IRowSet { const requestOptions = {} as google.bigtable.v2.IRowSet; if (request.rows && request.rows.rowRanges) { requestOptions.rowRanges = request.rows.rowRanges.map( From 07ec5655df0df11278110276116dafc8be7bd1e2 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Tue, 23 Apr 2024 17:21:14 -0400 Subject: [PATCH 12/15] Get rid of the any suppressed by typescript It was necessary to add guards to make this work. Also changed the error type to Google error. --- system-test/read-rows.ts | 58 ++++++++++++++++++++++++++++++---------- 1 file changed, 44 insertions(+), 14 deletions(-) diff --git a/system-test/read-rows.ts b/system-test/read-rows.ts index 482ae7a85..34b54028b 100644 --- a/system-test/read-rows.ts +++ b/system-test/read-rows.ts @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {Bigtable, protos, Table} from '../src'; +import {Bigtable, Cluster, protos, Table} from '../src'; const {tests} = require('../../system-test/data/read-rows-retry-test.json') as { tests: ReadRowsTest[]; }; @@ -20,7 +20,7 @@ import {google} from '../protos/protos'; import * as assert from 'assert'; import {describe, it, before} from 'mocha'; import {ReadRowsTest} from './testTypes'; -import {ServiceError, GrpcClient, CallOptions} from 'google-gax'; +import {ServiceError, GrpcClient, CallOptions, GoogleError} from 'google-gax'; import {MockServer} from '../src/util/mock-servers/mock-server'; import {MockService} from '../src/util/mock-servers/mock-service'; import {BigtableClientMockService} from '../src/util/mock-servers/service-implementations/bigtable-client-mock-service'; @@ -38,18 +38,41 @@ function rowResponseFromServer(rowKey: string) { }; } +function isRowKeysWithFunction(array: unknown): array is RowKeysWithFunction { + return (array as RowKeysWithFunction).asciiSlice !== undefined; +} + +function isRowKeysWithFunctionArray( + array: unknown[] +): array is RowKeysWithFunction[] { + return array.every((element: unknown) => { + return isRowKeysWithFunction(element); + }); +} + +interface TestRowRange { + startKey?: 'startKeyClosed' | 'startKeyOpen'; + endKey?: 'endKeyOpen' | 'endKeyClosed'; + startKeyClosed?: Uint8Array | string | null; + startKeyOpen?: Uint8Array | string | null; + endKeyOpen?: Uint8Array | string | null; + endKeyClosed?: Uint8Array | string | null; +} +interface RowKeysWithFunction { + asciiSlice: () => Uint8Array; +} function getRequestOptions(request: { - rows: { - rowRanges: google.bigtable.v2.RowRange[]; - rowKeys: Uint8Array[]; - }; - rowsLimit: string; + rows?: { + rowRanges?: TestRowRange[] | null; + rowKeys?: Uint8Array[] | null; + } | null; + rowsLimit?: string | number | Long | null | undefined; }): google.bigtable.v2.IRowSet { const requestOptions = {} as google.bigtable.v2.IRowSet; if (request.rows && request.rows.rowRanges) { requestOptions.rowRanges = request.rows.rowRanges.map( // eslint-disable-next-line @typescript-eslint/no-explicit-any - (range: google.bigtable.v2.RowRange) => { + (range: TestRowRange) => { const convertedRowRange = {} as {[index: string]: string}; { // startKey and endKey get filled in during the grpc request. @@ -69,10 +92,13 @@ function getRequestOptions(request: { } ); } - if (request.rows && request.rows.rowKeys) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - requestOptions.rowKeys = request.rows.rowKeys.map((rowKeys: any) => - rowKeys.asciiSlice() + if ( + request.rows && + request.rows.rowKeys && + isRowKeysWithFunctionArray(request.rows.rowKeys) + ) { + requestOptions.rowKeys = request.rows.rowKeys.map( + (rowKeys: RowKeysWithFunction) => rowKeys.asciiSlice() ); } // The grpc protocol sets rowsLimit to '0' if rowsLimit is not provided in the @@ -81,7 +107,11 @@ function getRequestOptions(request: { // Do not append rowsLimit to collection of request options if received grpc // rows limit is '0' so that test data in read-rows-retry-test.json remains // shorter. - if (request.rowsLimit && request.rowsLimit !== '0') { + if ( + request.rowsLimit && + request.rowsLimit !== '0' && + typeof request.rowsLimit === 'string' + ) { // eslint-disable-next-line @typescript-eslint/no-explicit-any (requestOptions as any).rowsLimit = parseInt(request.rowsLimit); } @@ -204,7 +234,7 @@ describe('Bigtable/Table', () => { } if (response.end_with_error) { // eslint-disable-next-line @typescript-eslint/no-explicit-any - const error: any = new Error(); + const error: GoogleError = new GoogleError(); error.code = response.end_with_error; stream.emit('error', error); } else { From 85a0317497b2f20d7d1b7909dc8e5b00c8ae57da Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Tue, 23 Apr 2024 17:35:47 -0400 Subject: [PATCH 13/15] Should use CreateReadStreamRequest as the type --- system-test/read-rows.ts | 13 ++++++------- system-test/testTypes.ts | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/system-test/read-rows.ts b/system-test/read-rows.ts index 34b54028b..b83f84c1b 100644 --- a/system-test/read-rows.ts +++ b/system-test/read-rows.ts @@ -19,7 +19,7 @@ const {tests} = require('../../system-test/data/read-rows-retry-test.json') as { import {google} from '../protos/protos'; import * as assert from 'assert'; import {describe, it, before} from 'mocha'; -import {ReadRowsTest} from './testTypes'; +import {CreateReadStreamRequest, ReadRowsTest} from './testTypes'; import {ServiceError, GrpcClient, CallOptions, GoogleError} from 'google-gax'; import {MockServer} from '../src/util/mock-servers/mock-server'; import {MockService} from '../src/util/mock-servers/mock-service'; @@ -59,7 +59,7 @@ interface TestRowRange { endKeyClosed?: Uint8Array | string | null; } interface RowKeysWithFunction { - asciiSlice: () => Uint8Array; + asciiSlice: () => string; } function getRequestOptions(request: { rows?: { @@ -67,8 +67,8 @@ function getRequestOptions(request: { rowKeys?: Uint8Array[] | null; } | null; rowsLimit?: string | number | Long | null | undefined; -}): google.bigtable.v2.IRowSet { - const requestOptions = {} as google.bigtable.v2.IRowSet; +}): CreateReadStreamRequest { + const requestOptions = {} as CreateReadStreamRequest; if (request.rows && request.rows.rowRanges) { requestOptions.rowRanges = request.rows.rowRanges.map( // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -113,7 +113,7 @@ function getRequestOptions(request: { typeof request.rowsLimit === 'string' ) { // eslint-disable-next-line @typescript-eslint/no-explicit-any - (requestOptions as any).rowsLimit = parseInt(request.rowsLimit); + requestOptions.rowsLimit = parseInt(request.rowsLimit); } return requestOptions; } @@ -125,7 +125,6 @@ describe('Bigtable/Table', () => { (bigtable as any).grpcCredentials = grpc.credentials.createInsecure(); const INSTANCE = bigtable.instance('instance'); - const TABLE = INSTANCE.table('table'); describe('close', () => { it('should fail when invoking readRows with closed client', async () => { @@ -192,7 +191,7 @@ describe('Bigtable/Table', () => { // These variables store request/response data capturing data sent // and received when using readRows with retries. This data is evaluated // in checkResults at the end of the test for correctness. - const requestedOptions: google.bigtable.v2.IRowSet[] = []; + const requestedOptions: CreateReadStreamRequest[] = []; const responses = test.responses; const rowKeysRead: string[][] = []; let endCalled = false; diff --git a/system-test/testTypes.ts b/system-test/testTypes.ts index 50d785c5d..d6b83464a 100644 --- a/system-test/testTypes.ts +++ b/system-test/testTypes.ts @@ -53,7 +53,7 @@ interface CreateReadStreamResponse { end_with_error?: 4; } -interface CreateReadStreamRequest { +export interface CreateReadStreamRequest { rowKeys: string[]; rowRanges: google.bigtable.v2.IRowRange[]; rowsLimit?: number; From 286e782ac1b1499be78afb297710dd63bbd2d8c4 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Wed, 24 Apr 2024 10:13:49 -0400 Subject: [PATCH 14/15] Remove the comment disabling linting on any --- system-test/read-rows.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/system-test/read-rows.ts b/system-test/read-rows.ts index b83f84c1b..93ca839b7 100644 --- a/system-test/read-rows.ts +++ b/system-test/read-rows.ts @@ -71,7 +71,6 @@ function getRequestOptions(request: { const requestOptions = {} as CreateReadStreamRequest; if (request.rows && request.rows.rowRanges) { requestOptions.rowRanges = request.rows.rowRanges.map( - // eslint-disable-next-line @typescript-eslint/no-explicit-any (range: TestRowRange) => { const convertedRowRange = {} as {[index: string]: string}; { @@ -112,7 +111,6 @@ function getRequestOptions(request: { request.rowsLimit !== '0' && typeof request.rowsLimit === 'string' ) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any requestOptions.rowsLimit = parseInt(request.rowsLimit); } return requestOptions; @@ -232,7 +230,6 @@ describe('Bigtable/Table', () => { }); } if (response.end_with_error) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any const error: GoogleError = new GoogleError(); error.code = response.end_with_error; stream.emit('error', error); From f4136c1f442ca9c722ecfdc4ae641cdc5d696b7b Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Wed, 24 Apr 2024 10:15:43 -0400 Subject: [PATCH 15/15] Remove the import. It is not used anymore. --- system-test/read-rows.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/system-test/read-rows.ts b/system-test/read-rows.ts index 93ca839b7..7deccc643 100644 --- a/system-test/read-rows.ts +++ b/system-test/read-rows.ts @@ -16,7 +16,6 @@ import {Bigtable, Cluster, protos, Table} from '../src'; const {tests} = require('../../system-test/data/read-rows-retry-test.json') as { tests: ReadRowsTest[]; }; -import {google} from '../protos/protos'; import * as assert from 'assert'; import {describe, it, before} from 'mocha'; import {CreateReadStreamRequest, ReadRowsTest} from './testTypes';