From 86f99f59ac530f2ead89ce817f5e20b47d28728d Mon Sep 17 00:00:00 2001 From: danieljbruce Date: Wed, 18 Dec 2024 17:07:39 -0500 Subject: [PATCH] chore(testproxy): Change the checkAndMutateRow test proxy service so that it uses the handwritten layer (#1541) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Build the request in a separate function * Apply the right mocks to the tests * Setup the frame for the new timeout test * Make sure to return the stream * Add assert checks - call count is 11 * Revert "Add assert checks - call count is 11" This reverts commit 24e3e334a8d884ffb6778dcc870d6e3e0a1d02cf. * Revert "Make sure to return the stream" This reverts commit 768242a3ee175d630ebf07f25350a2ae1db3f547. * Revert "Setup the frame for the new timeout test" This reverts commit f854546f9a814f7d988172afcd23aa363a6bf3a2. * Try to generate inverses * More code trying to invert the mutations * Delete files that are not needed anymore * Create a method for mutate.parse inverses * Create a method for createFlatMutationsListWithFn * Omit AppProfileId from call * Add a TODO for the other mutation types * Rename the file * Should invert mutations properly * Create some copy/pasted functions and a blank file * Remove rowKey from the inverse function * Remove unused row key * Rename mutations to entries * Create tests for the flatten mutations code * Add comment * Remove unused code * Set up the test proxy to use the handwritten layer * Fix the require errors * Remove a known failure that has been addressed * Add a test to ensure Gapic request is preserved * Remove some unused imports * Remove only everywhere * revert changes in the source file * Remove the mocks from the test file * This file does not need to live here anymore. * Add headers * Clarify the TODO * Simplify the variable name * Generate documentation for handwrittenLayerMutations * Address td-ignore statements * Use the callback provided to the Gapic layer * Remove onlys * Don’t modify mutation in the tests * Add a known failure * Add another known failure * Added more TestCheckAndMutateRow failures * Indent this code to indicate it is separate * Move inside the block * Refactor out the clientId string * Function renames * Prefer inline variable name * Better variable names * Eliminate use of any * Eliminate intermediate variable * Eliminate a line of code. Make variable inline. * Inline another variable * Shorten another test * Remove unused import * Shorten imports * Revert "Shorten imports" This reverts commit 4bb1bbe25e5ba4da23a57fde0cfc10370498a8f9. * Revert "Remove unused import" This reverts commit 554ba8d39845e4f97cdabe7c7811d712c91eeff5. * Revert "Revert "Remove unused import"" This reverts commit b14f66fabebde6937a419d5f6b18d4e72b8e6bb3. * Use service appProfileId by default * Eliminate large comment block * Address the TODOs - remove them * linter * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * Remove test * Add js suffix * Compile the test proxy folder * Add all testproxy typescript files to compiler --------- Co-authored-by: Kevin Kim Co-authored-by: Owl Bot --- test/mutation.ts | 4 +- test/test-proxy/checkAndMutateRowService.ts | 159 ++++++++++++++++++ test/test-proxy/mutationParseInverse.ts | 46 +++++ test/test-proxy/readModifyWriteRowService.ts | 24 ++- testproxy/known_failures.txt | 6 +- testproxy/services/check-and-mutate-row.js | 93 +++++++--- .../utils/request/createFlatMutationsList.ts | 73 ++++++++ .../services/utils/request/mutateInverse.ts | 97 +++++++++++ tsconfig.json | 4 +- 9 files changed, 479 insertions(+), 27 deletions(-) create mode 100644 test/test-proxy/checkAndMutateRowService.ts create mode 100644 test/test-proxy/mutationParseInverse.ts create mode 100644 testproxy/services/utils/request/createFlatMutationsList.ts create mode 100644 testproxy/services/utils/request/mutateInverse.ts diff --git a/test/mutation.ts b/test/mutation.ts index 7566a7496..a45c136b5 100644 --- a/test/mutation.ts +++ b/test/mutation.ts @@ -494,10 +494,10 @@ describe('Bigtable/Mutation', () => { data: [], }; const mutation = new Mutation(data); - Mutation.encodeSetCell = _data => { + sandbox.stub(Mutation, 'encodeSetCell').callsFake(_data => { assert.strictEqual(_data, data.data); return fakeEncoded; - }; + }); const mutationProto = mutation.toProto(); assert.strictEqual(mutationProto.mutations, fakeEncoded); assert.strictEqual(mutationProto.rowKey, data.key); diff --git a/test/test-proxy/checkAndMutateRowService.ts b/test/test-proxy/checkAndMutateRowService.ts new file mode 100644 index 000000000..9946c6612 --- /dev/null +++ b/test/test-proxy/checkAndMutateRowService.ts @@ -0,0 +1,159 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import * as assert from 'assert'; + +import {describe} from 'mocha'; +import {protos} from '../../src'; +import {BigtableClient} from '../../src/v2'; +import type {Callback, CallOptions, ServiceError} from 'google-gax'; +const checkAndMutateRowService = require('../../../testproxy/services/check-and-mutate-row.js'); +const createClient = require('../../../testproxy/services/create-client.js'); + +describe('TestProxy/CheckAndMutateRow', () => { + const testCases: protos.google.bigtable.v2.ICheckAndMutateRowRequest[] = [ + { + tableName: 'projects/projectId/instances/instance/tables/test-table', + appProfileId: 'test-app-profile', + rowKey: Buffer.from('test-row-key'), + predicateFilter: null, + trueMutations: [ + { + setCell: { + familyName: 'cf1', + timestampMicros: 1000007, + columnQualifier: Buffer.from('cq1'), + value: Buffer.from('value1'), + }, + }, + { + setCell: { + familyName: 'cf2', + timestampMicros: 1000007, + columnQualifier: Buffer.from('cq2'), + value: Buffer.from('value2'), + }, + }, + ], + falseMutations: [ + { + setCell: { + familyName: 'cf1', + timestampMicros: 1000007, + columnQualifier: Buffer.from('cq1'), + value: Buffer.from('value1'), + }, + }, + { + setCell: { + familyName: 'cf2', + timestampMicros: 1000007, + columnQualifier: Buffer.from('cq2'), + value: Buffer.from('value2'), + }, + }, + ], + }, + ]; + describe('Ensure the proper request is passed to the Gapic Layer', () => { + const clientId = 'TestCheckAndMutateRow_NoRetry_TransientError'; + testCases.forEach((checkAndMutateRowRequest, index) => { + it(`Run test ${index}`, done => { + (async () => { + const clientMap = new Map(); + const createClientFunction = createClient({clientMap}); + await new Promise((resolve, reject) => { + createClientFunction( + { + request: { + clientId, + dataTarget: 'localhost:1234', + projectId: 'projectId', + instanceId: 'instance', + appProfileId: '', + }, + }, + (error: ServiceError, response: {}) => { + if (error) { + reject(error); + } + resolve(response); + } + ); + }); + { + // Mock out the Gapic layer so we can see requests coming into it + const bigtable = clientMap.get(clientId); + const bigtableClient = new BigtableClient( + bigtable.options.BigtableClient + ); + bigtable.api['BigtableClient'] = bigtableClient; + bigtableClient.checkAndMutateRow = ( + request?: protos.google.bigtable.v2.ICheckAndMutateRowRequest, + optionsOrCallback?: + | CallOptions + | Callback< + protos.google.bigtable.v2.ICheckAndMutateRowResponse, + | protos.google.bigtable.v2.ICheckAndMutateRowRequest + | null + | undefined, + {} | null | undefined + >, + callback?: Callback< + protos.google.bigtable.v2.ICheckAndMutateRowResponse, + | protos.google.bigtable.v2.ICheckAndMutateRowRequest + | null + | undefined, + {} | null | undefined + > + ) => { + try { + // If the Gapic request is correct then the test passes. + assert.deepStrictEqual(request, checkAndMutateRowRequest); + } catch (e) { + // If the Gapic request is incorrect then the test fails with an error. + done(e); + } + if (callback) { + callback(null, {}); + } + return new Promise(resolve => { + const response: protos.google.bigtable.v2.ICheckAndMutateRowResponse = + {}; + resolve([response, {}, undefined]); + }); + }; + } + await new Promise((resolve, reject) => { + checkAndMutateRowService({clientMap})( + { + request: { + clientId, + request: checkAndMutateRowRequest, + }, + }, + (error: ServiceError, response: {}) => { + if (error) { + reject(error); + } + resolve(response); + } + ); + }); + done(); + })(); + }); + }); + }); +}); diff --git a/test/test-proxy/mutationParseInverse.ts b/test/test-proxy/mutationParseInverse.ts new file mode 100644 index 000000000..fd9dcbfa6 --- /dev/null +++ b/test/test-proxy/mutationParseInverse.ts @@ -0,0 +1,46 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import * as assert from 'assert'; +import {Mutation} from '../../src/mutation'; +import {mutationParseInverse} from '../../testproxy/services/utils/request/mutateInverse'; + +describe('Check mutation parse and mutationParseInverse are inverses', () => { + it('should invert mutations properly', () => { + const gapicLayerRequest = { + mutations: [ + { + setCell: { + familyName: 'cf1', + timestampMicros: 1000007, + columnQualifier: Buffer.from('cq1'), + value: Buffer.from('value1'), + }, + }, + { + setCell: { + familyName: 'cf2', + timestampMicros: 1000007, + columnQualifier: Buffer.from('cq2'), + value: Buffer.from('value2'), + }, + }, + ], + }; + assert.deepStrictEqual( + Mutation.parse(mutationParseInverse(gapicLayerRequest)), + gapicLayerRequest + ); + }); +}); diff --git a/test/test-proxy/readModifyWriteRowService.ts b/test/test-proxy/readModifyWriteRowService.ts index 474d25eab..de1abfda9 100644 --- a/test/test-proxy/readModifyWriteRowService.ts +++ b/test/test-proxy/readModifyWriteRowService.ts @@ -17,6 +17,7 @@ import * as assert from 'assert'; import {describe} from 'mocha'; import {protos} from '../../src'; import {BigtableClient} from '../../src/v2'; +import type {Callback, CallOptions} from 'google-gax'; const readModifyWriteRowService = require('../../../testproxy/services/read-modify-write-row.js'); const createClient = require('../../../testproxy/services/create-client.js'); @@ -74,16 +75,34 @@ describe('TestProxy/ReadModifyWriteRow', () => { ); bigtable.api['BigtableClient'] = bigtableClient; bigtableClient.readModifyWriteRow = ( - request?: protos.google.bigtable.v2.IReadModifyWriteRowRequest + request?: protos.google.bigtable.v2.IReadModifyWriteRowRequest, + optionsOrCallback?: + | CallOptions + | Callback< + protos.google.bigtable.v2.IReadModifyWriteRowResponse, + | protos.google.bigtable.v2.IReadModifyWriteRowRequest + | null + | undefined, + {} | null | undefined + >, + callback?: Callback< + protos.google.bigtable.v2.IReadModifyWriteRowResponse, + | protos.google.bigtable.v2.IReadModifyWriteRowRequest + | null + | undefined, + {} | null | undefined + > ) => { try { // If the Gapic request is correct then the test passes. assert.deepStrictEqual(request, readModifyWriteRowRequest); - done(); } catch (e) { // If the Gapic request is incorrect then the test fails with an error. done(e); } + if (callback) { + callback(null, {}); + } return new Promise(resolve => { const response: protos.google.bigtable.v2.IReadModifyWriteRowResponse = {}; @@ -109,6 +128,7 @@ describe('TestProxy/ReadModifyWriteRow', () => { } ); }); + done(); })(); }); }); diff --git a/testproxy/known_failures.txt b/testproxy/known_failures.txt index 95b03e50d..a370863d2 100644 --- a/testproxy/known_failures.txt +++ b/testproxy/known_failures.txt @@ -15,7 +15,11 @@ TestReadRows_Retry_WithRoutingCookie\| TestReadRows_Retry_WithRoutingCookie_MultipleErrorResponses\| TestReadRows_Retry_WithRetryInfo\| TestReadRows_Retry_WithRetryInfo_MultipleErrorResponse\| -TestCheckAndMutateRow_NoRetry_TransientError\| TestCheckAndMutateRow_Generic_DeadlineExceeded\| +TestCheckAndMutateRow_Generic_Headers\| +TestCheckAndMutateRow_NoRetry_TrueMutations\| +TestCheckAndMutateRow_NoRetry_FalseMutations\| +TestCheckAndMutateRow_Generic_CloseClient\| +TestCheckAndMutateRow_Generic_MultiStreams\| TestSampleRowKeys_Generic_DeadlineExceeded\| TestSampleRowKeys_Retry_WithRoutingCookie diff --git a/testproxy/services/check-and-mutate-row.js b/testproxy/services/check-and-mutate-row.js index 0f9474249..efbcf6d4f 100644 --- a/testproxy/services/check-and-mutate-row.js +++ b/testproxy/services/check-and-mutate-row.js @@ -16,32 +16,83 @@ const grpc = require('@grpc/grpc-js'); const normalizeCallback = require('./utils/normalize-callback.js'); +const getTableInfo = require('./utils/get-table-info'); +const { + createFlatMutationsListWithFnInverse, +} = require('../../build/testproxy/services/utils/request/createFlatMutationsList.js'); +const { + mutationParseInverse, +} = require('../../build/testproxy/services/utils/request/mutateInverse.js'); -const v2 = Symbol.for('v2'); +/** + * Transforms mutations from the gRPC layer format to the handwritten layer format. + * This function takes an array of gRPC layer mutations (google.bigtable.v2.IMutation[]) and converts + * them back to the format used by the handwritten layer. It essentially reverses the transformation + * performed by Mutation.parse. It's used internally by the test proxy for checkAndMutateRow. + * + * @param {google.bigtable.v2.IMutation[]} gapicLayerMutations An array of mutations in the gRPC layer format. + * @returns {FilterConfigOption[]} An array of mutations in the handwritten layer format. + */ +function handwrittenLayerMutations(gapicLayerMutations) { + return createFlatMutationsListWithFnInverse( + [ + { + mutations: gapicLayerMutations, + }, + ], + mutationParseInverse, + 1 + ); +} + +/** + * Converts a byte array (or string) back to a string. This is the inverse of + * Mutation.convertToBytes. + * + * @param {Bytes} bytes The byte array or string to convert. + * @returns {string} The converted string. + */ +function convertFromBytes(bytes) { + if (bytes instanceof Buffer) { + return bytes.toString(); + } else if (typeof bytes === 'string') { + return bytes; + } else { + throw new Error('Invalid input type. Must be Buffer or string.'); + } +} const checkAndMutateRow = ({clientMap}) => normalizeCallback(async rawRequest => { const {request} = rawRequest; - const {request: checkAndMutateRequest} = request; - const {falseMutations, rowKey, tableName, trueMutations} = - checkAndMutateRequest; - - const {clientId} = request; - const client = clientMap.get(clientId)[v2]; - const appProfileId = clientMap.get(clientId).appProfileId; - - const [result] = await client.checkAndMutateRow({ - appProfileId, - falseMutations, - rowKey, - tableName, - trueMutations, - }); - - return { - status: {code: grpc.status.OK, details: []}, - result, - }; + const {clientId, request: checkAndMutateRowRequest} = request; + const {appProfileId, falseMutations, rowKey, tableName, trueMutations} = + checkAndMutateRowRequest; + const onMatch = handwrittenLayerMutations(trueMutations); + const onNoMatch = handwrittenLayerMutations(falseMutations); + const id = convertFromBytes(rowKey); + const bigtable = clientMap.get(clientId); + bigtable.appProfileId = + appProfileId === '' ? clientMap.get(clientId).appProfileId : appProfileId; + const table = getTableInfo(bigtable, tableName); + const row = table.row(id); + const filter = []; + const filterConfig = {onMatch, onNoMatch}; + try { + const result = await row.filter(filter, filterConfig); + return { + status: {code: grpc.status.OK, details: []}, + row: result, + }; + } catch (e) { + return { + status: { + code: e.code, + details: [], + message: e.message, + }, + }; + } }); module.exports = checkAndMutateRow; diff --git a/testproxy/services/utils/request/createFlatMutationsList.ts b/testproxy/services/utils/request/createFlatMutationsList.ts new file mode 100644 index 000000000..7a074f18d --- /dev/null +++ b/testproxy/services/utils/request/createFlatMutationsList.ts @@ -0,0 +1,73 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import {FilterConfigOption} from '../../../../src/row'; +import {Mutation} from '../../../../src/mutation'; +import arrify = require('arrify'); + +/** + * Creates a flattened list of mutations by applying a transformation function to each entry in an array of FilterConfigOptions. + * + * This function takes an array of `FilterConfigOption` objects and a transformation function `f`. It applies `f` to each + * `FilterConfigOption` (after casting it to a `Mutation` object) and extracts the `mutations` property from the result of `f`. + * The `mutations` property is expected to be an array of type `T`, which extends `google.bigtable.v2.IMutation`. + * Finally, it flattens the resulting array of `T[]` into a single, concatenated array of `T`. + * + * @template T The type of mutation objects within the `mutations` array returned by `f`. Must extend `google.bigtable.v2.IMutation`. + * @param {FilterConfigOption[]} entries An array of `FilterConfigOption` objects, each representing a set of mutations. + * @param {function(Mutation): {mutations: T[]}} f The transformation function to apply to each `FilterConfigOption`. + * This function takes a `Mutation` object as input and must return an object with a `mutations` property that is an array of `T` objects. + * @returns {T[]} A flattened array of mutations of type `T`, created by concatenating the `mutations` arrays returned by applying `f` to each entry. + */ +export function createFlatMutationsListWithFn( + entries: FilterConfigOption[], + f: (entry: Mutation) => {mutations: T[]} +) { + const e2 = arrify(entries).map(entry => f(entry as Mutation).mutations!); + return e2.reduce((a, b) => a.concat(b), []); +} + +/** + * Partially inverts createFlatMutationsListWithFn, reconstructing the original + * FilterConfigOption[] by assuming 'f' converts Mutations to their protobuf + * representation. Note: This does *not* invert the transformation performed by 'f' itself. + * + * @param {T[]} entries The flattened mutations list. + * @param {Function} fInverse The inverse of the function 'f' used in createFlatMutationsListWithFn. MUST BE PROVIDED and MUST correctly invert 'f'. + * @param {number} numEntries The original number of entries. This is REQUIRED as the flattening operation loses this information. + * @returns {FilterConfigOption[]} The reconstructed FilterConfigOption array. + */ +export function createFlatMutationsListWithFnInverse( + entries: T[], + fInverse: (entry: T) => Mutation, + numEntries: number +): FilterConfigOption[] { + const invertedEntries: FilterConfigOption[] = []; + const mutationsPerEntry = entries.length / numEntries; + + for (let i = 0; i < numEntries; i++) { + const start = i * mutationsPerEntry; + const end = start + mutationsPerEntry; + const entryMutations = entries.slice(start, end) as unknown as T[]; // Type cast to align with fInverse input + + const invertedEntry: FilterConfigOption = {}; + for (const mutation of entryMutations) { + Object.assign(invertedEntry, fInverse(mutation)); // Apply inverse function to each mutation + } + + invertedEntries.push(invertedEntry); + } + + return invertedEntries; +} diff --git a/testproxy/services/utils/request/mutateInverse.ts b/testproxy/services/utils/request/mutateInverse.ts new file mode 100644 index 000000000..899b8bf48 --- /dev/null +++ b/testproxy/services/utils/request/mutateInverse.ts @@ -0,0 +1,97 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import {Data, Mutation, Bytes} from '../../../../src/mutation'; +import * as protos from '../../../../protos/protos'; + +/** + * Inverts the Mutation.parse function. Reconstructs a Mutation object from its + * protobuf representation. + * + * @param {protos.google.bigtable.v2.IMutateRowRequest} req The protobuf representation of the mutation. + * @returns {Mutation} The reconstructed Mutation object. + */ +export function mutationParseInverse(req: { + mutations: protos.google.bigtable.v2.IMutation[]; +}): Mutation { + let method: string | undefined; + let data: Data; + + if (req.mutations && req.mutations.length > 0) { + if (req.mutations[0].setCell) { + method = Mutation.methods.INSERT; + const localData = {} as any; + + req.mutations.forEach(m => { + const cell = m.setCell; + if (cell) { + const family = cell.familyName!; + const qualifier = Mutation.convertFromBytes( + cell.columnQualifier! as Bytes + ); + + // Now TypeScript knows that 'data' is an object, and 'family' is a string key + if (!localData[family]) { + localData[family] = {}; + } + localData[family][qualifier as string] = { + value: Mutation.convertFromBytes(cell?.value as Bytes), + timestamp: cell?.timestampMicros, + }; + } + }); + data = localData; + } else if ( + req.mutations.some( + m => m.deleteFromColumn || m.deleteFromFamily || m.deleteFromRow + ) + ) { + method = Mutation.methods.DELETE; + const localData: Data[] = []; + + req.mutations.forEach(m => { + if (m.deleteFromColumn) { + const col = m.deleteFromColumn; + + const column = { + family: col?.familyName, + qualifier: Mutation.convertFromBytes(col?.columnQualifier as Bytes), + }; + + const qualifier = `${column.family}:${column.qualifier}`; + + if ( + col?.timeRange?.startTimestampMicros && + col?.timeRange.endTimestampMicros + ) { + const time = { + start: col?.timeRange?.startTimestampMicros, + end: col?.timeRange.endTimestampMicros, + }; + localData.push({column: qualifier, time}); + } else { + localData.push(qualifier); + } + } else if (m.deleteFromFamily) { + localData.push(m.deleteFromFamily?.familyName); + } else if (m.deleteFromRow) { + localData.push({}); // Represent deleteFromRow as an empty object + } + }); + data = localData; + } + } + + return {method: method!, data} as Mutation; // method cannot be undefined here, assert non-null +} diff --git a/tsconfig.json b/tsconfig.json index c78f1c884..15d0354e8 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -14,6 +14,8 @@ "src/**/*.ts", "test/*.ts", "test/**/*.ts", - "system-test/*.ts" + "system-test/*.ts", + "testproxy/*.ts", + "testproxy/**/*.ts" ] }