Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
56aa444
Use prettyPrintRequest from readRowsImpl1
danieljbruce Jul 16, 2024
0699bf7
Define interfaces for creating a Bigtable service
danieljbruce Jul 16, 2024
f77261a
Remove keyFrom and keyTo from each test
danieljbruce Jul 16, 2024
958a255
Define the service inline for standard keys errors
danieljbruce Jul 16, 2024
5558d00
Add a comment about the refactor steps I am doing
danieljbruce Jul 16, 2024
7589b60
Add a header to the service parameters file
danieljbruce Jul 16, 2024
ec545e9
Use the ChunkGeneratorParameters interface
danieljbruce Jul 16, 2024
15479c9
Simplify the diff by creating local variables
danieljbruce Jul 16, 2024
588234a
Remove comment
danieljbruce Jul 16, 2024
de7d357
Eliminate chunks per response constant
danieljbruce Jul 16, 2024
228afb1
Change imports
danieljbruce Jul 16, 2024
786fb66
Set up the second ReadRows service to use params
danieljbruce Jul 17, 2024
95fd859
Remove duplicate copy of generate chunks from serv
danieljbruce Jul 17, 2024
e2837a5
Remove second copy of isKeyInRowSet
danieljbruce Jul 17, 2024
c45b0bb
Eliminate duplicate copies of the debug log
danieljbruce Jul 17, 2024
af2955b
Fix a bug for the to and from service parameters
danieljbruce Jul 17, 2024
68e9d04
Add cancel back into the mock service for testing
danieljbruce Jul 17, 2024
a7e13df
Add variables to match service 1
danieljbruce Jul 17, 2024
5bd8a55
Add one more check to match the other service
danieljbruce Jul 17, 2024
e16b3c9
Remove usages of ReadRows2Impl
danieljbruce Jul 17, 2024
952a6a9
Remove the new service
danieljbruce Jul 17, 2024
38b0006
Moved the position of the comment for 150 rows
danieljbruce Jul 17, 2024
716008a
Eliminate setting keyTo and keyFrom
danieljbruce Jul 17, 2024
22848fe
Add a second test for an error at a random pos
danieljbruce Jul 17, 2024
917f3dc
Add some comments for documentation
danieljbruce Jul 17, 2024
799f5ef
Chunk generation - add the parameter
danieljbruce Jul 17, 2024
6be2743
Added comments to all the interfaces
danieljbruce Jul 17, 2024
8f9ffa0
Merge branch '3527322442-refactor-the-test-into-classes-2' of https:/…
danieljbruce Jul 22, 2024
ab81643
Merge branch '3527322442-to-main' of https://github.com/googleapis/no…
danieljbruce Jul 22, 2024
1b0dec8
Group the property getter into a function
danieljbruce Jul 22, 2024
51e9b85
Group key selection into functions
danieljbruce Jul 22, 2024
5418771
Fix typo: default key
danieljbruce Jul 22, 2024
ee03787
Documentation for isInRowSet
danieljbruce Jul 22, 2024
65644b1
Eliminate variable - move function inline
danieljbruce Jul 22, 2024
d8092e9
Omit optional selector on stream
danieljbruce Jul 22, 2024
afa3d86
Create ReadRowsWritableStream interface
danieljbruce Jul 22, 2024
4fdea53
Use new interface, remove ServerWritableStream
danieljbruce Jul 22, 2024
6a6ae48
Don’t pass the whole stream into helpers
danieljbruce Jul 22, 2024
7431ae1
Add a function for generating the chunks
danieljbruce Jul 22, 2024
8389b61
The debug log should be a pass through parameter
danieljbruce Jul 22, 2024
7e2a76c
Merge branch '3527322442-to-main-2' of https://github.com/googleapis/…
danieljbruce Jul 29, 2024
7162866
Solve compiler errors resulting immediately from
danieljbruce Jul 29, 2024
882913b
Add debugLog parameter to various functions
danieljbruce Jul 29, 2024
cefe405
Add return type
danieljbruce Jul 29, 2024
8681d45
Remove exports - functions are only used in this
danieljbruce Jul 29, 2024
5ea0cc0
Revise merge correction
danieljbruce Jul 29, 2024
f771841
Remove TODO for the stream type
danieljbruce Jul 29, 2024
25ca84f
Update the getKeyValue function
danieljbruce Jul 29, 2024
b907a31
Eliminate place where debug log is needed
danieljbruce Jul 29, 2024
ae3447c
Run linter
danieljbruce Jul 29, 2024
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
28 changes: 18 additions & 10 deletions test/readrows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,30 @@
// limitations under the License.

import {before, describe, it} from 'mocha';
import {Bigtable, protos, Row, Table} from '../src';
import {Bigtable, Row, Table} from '../src';
import * as assert from 'assert';
import {Transform, PassThrough, pipeline} from 'stream';

import {GoogleError} from 'google-gax';
import {MockServer} from '../src/util/mock-servers/mock-server';
import {BigtableClientMockService} from '../src/util/mock-servers/service-implementations/bigtable-client-mock-service';
import {MockService} from '../src/util/mock-servers/mock-service';
import {debugLog, readRowsImpl} from './utils/readRowsImpl';
import {ServerWritableStream} from '@grpc/grpc-js';
import {readRowsImpl} from './utils/readRowsImpl';

import {ReadRowsServiceParameters} from '../test/utils/readRowsServiceParameters';
import {
ReadRowsServiceParameters,
ReadRowsWritableStream,
} from '../test/utils/readRowsServiceParameters';
import * as mocha from 'mocha';

const DEBUG = process.env.BIGTABLE_TEST_DEBUG === 'true';

function debugLog(text: string) {
if (DEBUG) {
console.log(text);
}
}

Copy link
Contributor Author

@danieljbruce danieljbruce Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DebugLog is just being moved from the service to here in the test

// Define parameters for a standard Bigtable Mock service
const VALUE_SIZE = 1024 * 1024;
// we want each row to be split into 2 chunks of different sizes
Expand All @@ -41,16 +51,12 @@ const STANDARD_SERVICE_WITHOUT_ERRORS: ReadRowsServiceParameters = {
valueSize: VALUE_SIZE,
chunkSize: CHUNK_SIZE,
chunksPerResponse: CHUNKS_PER_RESPONSE,
debugLog,
};

type PromiseVoid = Promise<void>;
interface ServerImplementationInterface {
(
server: ServerWritableStream<
protos.google.bigtable.v2.IReadRowsRequest,
protos.google.bigtable.v2.IReadRowsResponse
>
): PromiseVoid;
(server: ReadRowsWritableStream): PromiseVoid;
}

describe('Bigtable/ReadRows', () => {
Expand Down Expand Up @@ -316,6 +322,7 @@ describe('Bigtable/ReadRows', () => {
chunkSize: CHUNK_SIZE,
chunksPerResponse: CHUNKS_PER_RESPONSE,
errorAfterChunkNo,
debugLog,
}) as ServerImplementationInterface,
});
let receivedRowCount = 0;
Expand Down Expand Up @@ -367,6 +374,7 @@ describe('Bigtable/ReadRows', () => {
valueSize: 1,
chunkSize: 1,
chunksPerResponse: 1,
debugLog,
}) as ServerImplementationInterface,
});
const sleep = (ms: number) => {
Expand Down
181 changes: 102 additions & 79 deletions test/utils/readRowsImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,25 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import {ServerWritableStream} from '@grpc/grpc-js';
import {protos} from '../../src';
import {GoogleError, Status} from 'google-gax';
import {
ChunkGeneratorParameters,
DebugLog,
ReadRowsServiceParameters,
ReadRowsWritableStream,
} from './readRowsServiceParameters';

const DEBUG = process.env.BIGTABLE_TEST_DEBUG === 'true';

export function debugLog(text: string) {
if (DEBUG) {
console.log(text);
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has just been moved to the test and will be passed into the service.

import {google} from '../../protos/protos';
import IRowRange = google.bigtable.v2.IRowRange;

// Generate documentation for this function
/** Pretty prints the request object.
* @param request The request object to pretty print.
* @param debugLog The logging function for printing test output.
*/
export function prettyPrintRequest(
request: protos.google.bigtable.v2.IReadRowsRequest
function prettyPrintRequest(
request: protos.google.bigtable.v2.IReadRowsRequest,
debugLog: DebugLog
) {
// pretty-printing important parts of the request.
// doing it field by field because we want to apply .toString() to all key fields
Expand Down Expand Up @@ -79,8 +76,13 @@ export function prettyPrintRequest(
* The fake table contains monotonically increasing zero padded rows
* in the range [keyFrom, keyTo).
* @param chunkGeneratorParameters The parameters for generating chunks.
* @param debugLog The logging function for printing test output.
* @returns {protos.google.bigtable.v2.ReadRowsResponse.ICellChunk[]} The generated chunks.
*/
function generateChunks(chunkGeneratorParameters: ChunkGeneratorParameters) {
function generateChunks(
chunkGeneratorParameters: ChunkGeneratorParameters,
debugLog: DebugLog
): protos.google.bigtable.v2.ReadRowsResponse.ICellChunk[] {
const keyFrom = chunkGeneratorParameters.keyFrom;
const keyTo = chunkGeneratorParameters.keyTo;
debugLog(`generating chunks from ${keyFrom} to ${keyTo}`);
Expand Down Expand Up @@ -125,6 +127,11 @@ function generateChunks(chunkGeneratorParameters: ChunkGeneratorParameters) {
return chunks;
}

/** Checks if the given key is in the provided RowSet.
* @param stringKey The key to check.
* @param rowSet The RowSet to check against.
* @returns {boolean} True if the key is in the RowSet, false otherwise.
*/
function isKeyInRowSet(
stringKey: string,
rowSet?: protos.google.bigtable.v2.IRowSet | null
Expand Down Expand Up @@ -163,6 +170,84 @@ function isKeyInRowSet(
return true;
}

/** Gets the key value for the given property specified in the request.
* @param request The request object to get the key value from.
* @param property The property from the request to get the value from.
* @returns {string | undefined} The key value from the request.
*/
function getKeyValue(
request: protos.google.bigtable.v2.IReadRowsRequest,
property: keyof IRowRange
) {
if (
request?.rows?.rowRanges &&
request?.rows?.rowRanges[0] &&
request?.rows?.rowRanges[0][property]?.toString()
) {
return request?.rows?.rowRanges[0][property]?.toString();
}
return undefined;
}

/** Gets the key from the request object.
* @param stream The stream object to get the key from.
* @param keySelectionParameters The parameters for selecting the key.
* @returns {number} The selected key for generating chunks
*/
function getSelectedKey(
request: protos.google.bigtable.v2.IReadRowsRequest,
keySelectionParameters: {
keyOpenProperty: keyof IRowRange;
keyClosedProperty: keyof IRowRange;
defaultKey?: number;
}
) {
const keyRequestOpen = getKeyValue(
request,
keySelectionParameters.keyOpenProperty
);
const keyRequestClosed = getKeyValue(
request,
keySelectionParameters.keyClosedProperty
);
const defaultKey = keySelectionParameters.defaultKey;
return defaultKey !== undefined
? defaultKey
: keyRequestClosed
? parseInt(keyRequestClosed as string)
: parseInt(keyRequestOpen as string) + 1;
}

/** Generates chunks for rows in a fake table that match the provided RowSet.
* The fake table contains monotonically increasing zero padded rows
* in the range [keyFrom, keyTo).
* @param request The request object to generate chunks from.
* @param serviceParameters The parameters for generating chunks.
* @returns {protos.google.bigtable.v2.ReadRowsResponse.ICellChunk[]} The generated chunks.
*/
function generateChunksFromRequest(
request: protos.google.bigtable.v2.IReadRowsRequest,
serviceParameters: ReadRowsServiceParameters
) {
return generateChunks(
{
keyFrom: getSelectedKey(request, {
keyOpenProperty: 'startKeyOpen',
keyClosedProperty: 'startKeyClosed',
defaultKey: serviceParameters.keyFrom,
}),
keyTo: getSelectedKey(request, {
keyOpenProperty: 'endKeyOpen',
keyClosedProperty: 'endKeyClosed',
defaultKey: serviceParameters.keyTo,
}),
chunkSize: serviceParameters.chunkSize,
valueSize: serviceParameters.valueSize,
},
serviceParameters.debugLog
);
}

// Returns an implementation of the server streaming ReadRows call that would return
// monotonically increasing zero padded rows in the range [keyFrom, keyTo).
// The returned implementation can be passed to gRPC server.
Expand All @@ -171,20 +256,11 @@ function isKeyInRowSet(
// TODO: Perhaps group the if statements into classes so that they can be unit tested.
export function readRowsImpl(
serviceParameters: ReadRowsServiceParameters
): (
stream: ServerWritableStream<
protos.google.bigtable.v2.IReadRowsRequest,
protos.google.bigtable.v2.IReadRowsResponse
>
) => Promise<void> {
): (stream: ReadRowsWritableStream) => Promise<void> {
let errorAfterChunkNo = serviceParameters.errorAfterChunkNo;
return async (
stream: ServerWritableStream<
protos.google.bigtable.v2.IReadRowsRequest,
protos.google.bigtable.v2.IReadRowsResponse
>
): Promise<void> => {
prettyPrintRequest(stream.request);
return async (stream: ReadRowsWritableStream): Promise<void> => {
const debugLog = serviceParameters.debugLog;
prettyPrintRequest(stream.request, debugLog);

let stopWaiting: () => void = () => {};
let cancelled = false;
Expand Down Expand Up @@ -231,60 +307,7 @@ export function readRowsImpl(
});

let chunksSent = 0;
let keyFromRequestClosed: string | undefined;
if (
stream?.request?.rows?.rowRanges &&
stream?.request?.rows?.rowRanges[0] &&
stream?.request?.rows?.rowRanges[0]?.startKeyClosed?.toString()
) {
keyFromRequestClosed =
stream?.request?.rows?.rowRanges[0]?.startKeyClosed?.toString();
}
let keyFromRequestOpen: string | undefined;
if (
stream?.request?.rows?.rowRanges &&
stream?.request?.rows?.rowRanges[0] &&
stream?.request?.rows?.rowRanges[0]?.startKeyOpen?.toString()
) {
keyFromRequestOpen =
stream?.request?.rows?.rowRanges[0]?.startKeyOpen?.toString();
}
let keyToRequestClosed: string | undefined;
if (
stream?.request?.rows?.rowRanges &&
stream?.request?.rows?.rowRanges[0] &&
stream?.request?.rows?.rowRanges[0]?.endKeyClosed?.toString()
) {
keyToRequestClosed =
stream?.request?.rows?.rowRanges[0]?.endKeyClosed?.toString();
}
let keyToRequestOpen: string | undefined;
if (
stream?.request?.rows?.rowRanges &&
stream?.request?.rows?.rowRanges[0] &&
stream?.request?.rows?.rowRanges[0]?.endKeyOpen?.toString()
) {
keyToRequestOpen =
stream?.request?.rows?.rowRanges[0]?.endKeyOpen?.toString();
}
const keyFromUsed =
serviceParameters.keyFrom !== undefined
? serviceParameters.keyFrom
: keyFromRequestClosed
? parseInt(keyFromRequestClosed as string)
: parseInt(keyFromRequestOpen as string) + 1;
const keyToUsed =
serviceParameters.keyTo !== undefined
? serviceParameters.keyTo
: keyToRequestClosed
? parseInt(keyToRequestClosed as string)
: parseInt(keyToRequestOpen as string) + 1;
const chunks = generateChunks({
keyFrom: keyFromUsed,
keyTo: keyToUsed,
chunkSize: serviceParameters.chunkSize,
valueSize: serviceParameters.valueSize,
});
const chunks = generateChunksFromRequest(stream.request, serviceParameters);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This single line of code is a substitute for all of the code deleted previous to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code for each if statement now lives in getKeyValue. generateChunksFromRequest calls generateChunks making 2 calls to getSelectedKey each making 2 calls to getKeyValue.

let lastScannedRowKey: string | undefined;
let currentResponseChunks: protos.google.bigtable.v2.ReadRowsResponse.ICellChunk[] =
[];
Expand Down
11 changes: 11 additions & 0 deletions test/utils/readRowsServiceParameters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import {ServerWritableStream} from '@grpc/grpc-js';
import {protos} from '../../src';

/**
* This file contains the parameters for the readRowsService.
*/
Expand All @@ -21,14 +24,22 @@ interface SharedReadRowsParameters {
valueSize: number; // An upper bound on the amount of data included in the chunks
}

export type DebugLog = (message: string) => void;

export interface ReadRowsServiceParameters extends SharedReadRowsParameters {
keyFrom?: number; // The key the data coming from the service will start from
keyTo?: number; // The key the data coming from the service will end at
errorAfterChunkNo?: number; // The chunk that the error should come after
chunksPerResponse: number; // The total number of chunks the server should send
debugLog: DebugLog;
}

export interface ChunkGeneratorParameters extends SharedReadRowsParameters {
keyFrom: number; // The first row in the generated chunks will start with this key
keyTo: number; // The last row in the generated chunks will start with this key
}

export type ReadRowsWritableStream = ServerWritableStream<
protos.google.bigtable.v2.IReadRowsRequest,
protos.google.bigtable.v2.IReadRowsResponse
>;