diff --git a/handwritten/spanner/observability-test/spanner.ts b/handwritten/spanner/observability-test/spanner.ts index 860c1d806518..db5fa03575f9 100644 --- a/handwritten/spanner/observability-test/spanner.ts +++ b/handwritten/spanner/observability-test/spanner.ts @@ -416,6 +416,44 @@ describe('EndToEnd', async () => { expectedEventNames, ); }); + + it('runPartitionedUpdate with string query', async () => { + await database.runPartitionedUpdate(updateSql); + + const expectedSpanNames = [ + 'CloudSpanner.Snapshot.begin', + 'CloudSpanner.Snapshot.runStream', + 'CloudSpanner.Snapshot.run', + 'CloudSpanner.Dml.runUpdate', + 'CloudSpanner.PartitionedDml.runUpdate', + 'CloudSpanner.Database.runPartitionedUpdate', + ]; + const expectedEventNames = [ + 'Begin Transaction', + 'Transaction Creation Done', + 'Starting stream', + 'Acquiring session', + 'Cache hit: has usable session', + 'Acquired session', + ]; + verifySpansAndEvents( + traceExporter, + expectedSpanNames, + expectedEventNames, + ); + + const finishedSpans = traceExporter.getFinishedSpans(); + for (const span of finishedSpans) { + const hasNumericAttributeKeys = Object.keys(span.attributes).some( + key => /^\d+$/.test(key), + ); + assert.strictEqual( + hasNumericAttributeKeys, + false, + `Span ${span.name} should not contain numeric attributes from string spreading`, + ); + } + }); }); }); }); @@ -843,6 +881,28 @@ describe('ObservabilityOptions injection and propagation', async () => { expectedEventNames, `Unexpected events:\n\tGot: ${actualEventNames}\n\tWant: ${expectedEventNames}`, ); + + for (const span of spansFromInjected) { + if ( + span.name === 'CloudSpanner.Database.run' || + span.name === 'CloudSpanner.Database.runStream' || + span.name === 'CloudSpanner.Snapshot.runStream' + ) { + assert.strictEqual( + span.attributes['db.statement'], + 'SELECT 1', + `Span ${span.name} should have db.statement set to 'SELECT 1'`, + ); + const hasNumericAttributeKeys = Object.keys(span.attributes).some( + key => /^\d+$/.test(key), + ); + assert.strictEqual( + hasNumericAttributeKeys, + false, + `Span ${span.name} should not contain numeric attributes from string spreading`, + ); + } + } } catch (err) { assert.ifError(err); } finally { diff --git a/handwritten/spanner/src/batch-transaction.ts b/handwritten/spanner/src/batch-transaction.ts index b7b72826a9ab..ac533865d77a 100644 --- a/handwritten/spanner/src/batch-transaction.ts +++ b/handwritten/spanner/src/batch-transaction.ts @@ -33,7 +33,12 @@ import { ResourceCallback, addLeaderAwareRoutingHeader, } from '../src/common'; -import {startTrace, setSpanError, traceConfig} from './instrument'; +import { + startTrace, + setSpanError, + traceConfig, + getQueryTraceConfig, +} from './instrument'; import {injectRequestIDIntoHeaders} from './request_id_header'; import {isString} from './helper'; @@ -169,15 +174,24 @@ class BatchTransaction extends Snapshot { const request: ExecuteSqlRequest = typeof query === 'string' ? {sql: query} : query; - const reqOpts = Object.assign({}, request, Snapshot.encodeParams(request)); + const { + gaxOptions: _omittedGaxOptions, + types: _omittedTypes, + ...cleanRequest + } = request as ExecuteSqlRequest & {types?: unknown}; + void _omittedGaxOptions; + void _omittedTypes; - delete (reqOpts as any).gaxOptions; - delete (reqOpts as any).types; + const reqOpts = Object.assign( + {}, + cleanRequest, + Snapshot.encodeParams(request), + ); const traceConfig: traceConfig = { - sql: request.sql, opts: this._observabilityOptions, dbName: this.getDBName(), + ...getQueryTraceConfig(query), }; return startTrace( 'BatchTransaction.createQueryPartitions', @@ -233,17 +247,21 @@ class BatchTransaction extends Snapshot { 'BatchTransaction.createPartitions_', traceConfig, span => { - const query = Object.assign({}, config.reqOpts, { + const baseRequest = Object.assign({}, config.reqOpts, { session: this.session.formattedName_, transaction: {id: this.id}, }); - config.reqOpts = Object.assign({}, query); + config.reqOpts = baseRequest; const headers = { [CLOUD_RESOURCE_HEADER]: (this.session.parent as Database) .formattedName_, }; config.headers = injectRequestIDIntoHeaders(headers, this.session); - delete query.partitionOptions; + const { + partitionOptions: _omittedPartitionOptions, + ...baseRequestWithoutPartitionOptions + } = baseRequest; + void _omittedPartitionOptions; this.session.request(config, (err, resp) => { if (err) { setSpanError(span, err); @@ -253,7 +271,11 @@ class BatchTransaction extends Snapshot { } const partitions = resp.partitions.map(partition => { - return Object.assign({}, query, partition); + return Object.assign( + {}, + baseRequestWithoutPartitionOptions, + partition, + ); }); if (resp.transaction) { @@ -323,14 +345,20 @@ class BatchTransaction extends Snapshot { 'BatchTransaction.createReadPartitions', traceConfig, span => { - const reqOpts = Object.assign({}, options, { + const { + gaxOptions: _omittedGaxOptions, + keys: _omittedKeys, + ranges: _omittedRanges, + ...cleanOptions + } = options; + void _omittedGaxOptions; + void _omittedKeys; + void _omittedRanges; + + const reqOpts = Object.assign({}, cleanOptions, { keySet: Snapshot.encodeKeySet(options), }); - delete reqOpts.gaxOptions; - delete reqOpts.keys; - delete reqOpts.ranges; - const headers: {[k: string]: string} = {}; if (this._getSpanner().routeToLeaderEnabled) { addLeaderAwareRoutingHeader(headers); diff --git a/handwritten/spanner/src/codec.ts b/handwritten/spanner/src/codec.ts index be713e7e130f..66a03628d5c7 100644 --- a/handwritten/spanner/src/codec.ts +++ b/handwritten/spanner/src/codec.ts @@ -1464,7 +1464,7 @@ interface FieldType extends Type { * // {type: 'float64'} * ``` */ -function getType(value: Value): Type { +function getType(value: Value, enableUuidAsUntyped?: boolean): Type { const isSpecialNumber = isInfinite(value) || (isNumber(value) && isNaN(value)); @@ -1512,7 +1512,12 @@ function getType(value: Value): Type { return {type: 'bool'}; } - if (process.env['SPANNER_ENABLE_UUID_AS_UNTYPED'] === 'true') { + const isUuidUntyped = + enableUuidAsUntyped !== undefined + ? enableUuidAsUntyped + : process.env['SPANNER_ENABLE_UUID_AS_UNTYPED'] === 'true'; + + if (isUuidUntyped) { if (!uuidUntypedFlagWarned) { process.emitWarning( 'SPANNER_ENABLE_UUID_AS_UNTYPED environment variable is deprecated and will be removed in a future release.', @@ -1545,7 +1550,7 @@ function getType(value: Value): Type { return { type: 'struct', fields: Array.from(value).map(({name, value}) => { - return Object.assign({name}, getType(value)); + return Object.assign({name}, getType(value, isUuidUntyped)); }), }; } @@ -1563,7 +1568,7 @@ function getType(value: Value): Type { return { type: 'array', - child: getType(child), + child: getType(child, isUuidUntyped), }; } diff --git a/handwritten/spanner/src/database.ts b/handwritten/spanner/src/database.ts index 315f3b478f55..00c6592ac074 100644 --- a/handwritten/spanner/src/database.ts +++ b/handwritten/spanner/src/database.ts @@ -112,6 +112,7 @@ import { setSpanError, setSpanErrorAndException, traceConfig, + getQueryTraceConfig, } from './instrument'; import { AtomicCounter, @@ -2905,8 +2906,8 @@ class Database extends common.GrpcServiceObject { return startTrace( 'Database.run', { - ...(query as ExecuteSqlRequest), ...this._traceConfig, + ...getQueryTraceConfig(query), }, span => { this.runStream(query, options) @@ -2959,10 +2960,8 @@ class Database extends common.GrpcServiceObject { return startTrace( 'Database.runPartitionedUpdate', { - ...(query as RunPartitionedUpdateOptions), ...this._traceConfig, - requestTag: (query as RunPartitionedUpdateOptions)?.requestOptions - ?.requestTag, + ...getQueryTraceConfig(query), }, span => { this.sessionFactory_.getSessionForPartitionedOps((err, session) => { @@ -3152,9 +3151,8 @@ class Database extends common.GrpcServiceObject { return startTrace( 'Database.runStream', { - ...(query as ExecuteSqlRequest), ...this._traceConfig, - requestTag: (query as ExecuteSqlRequest)?.requestOptions?.requestTag, + ...getQueryTraceConfig(query), }, span => { this.sessionFactory_.getSession((err, session) => { diff --git a/handwritten/spanner/src/instrument.ts b/handwritten/spanner/src/instrument.ts index f8a4e487d0f5..4fa05e2bc472 100644 --- a/handwritten/spanner/src/instrument.ts +++ b/handwritten/spanner/src/instrument.ts @@ -90,8 +90,37 @@ interface traceConfig { opts?: ObservabilityOptions; } +interface QueryWithRequestOptions { + sql?: string | SQLStatement; + requestOptions?: { + requestTag?: string | null; + transactionTag?: string | null; + } | null; +} + +function getQueryTraceConfig(query?: string | QueryWithRequestOptions | null): { + sql?: string | SQLStatement; + requestTag?: string | null; +} { + if (typeof query === 'string') { + return {sql: query}; + } + if (query && typeof query === 'object') { + return { + sql: query.sql, + requestTag: query.requestOptions?.requestTag, + }; + } + return {}; +} + const SPAN_NAMESPACE_PREFIX = 'CloudSpanner'; // TODO: discuss & standardize this prefix. -export {SPAN_NAMESPACE_PREFIX, traceConfig}; +export { + SPAN_NAMESPACE_PREFIX, + traceConfig, + QueryWithRequestOptions, + getQueryTraceConfig, +}; const { AsyncHooksContextManager, diff --git a/handwritten/spanner/src/transaction.ts b/handwritten/spanner/src/transaction.ts index bbd87d49fa9e..afac6a40063e 100644 --- a/handwritten/spanner/src/transaction.ts +++ b/handwritten/spanner/src/transaction.ts @@ -39,6 +39,7 @@ import { setSpanError, setSpanErrorAndException, traceConfig, + getQueryTraceConfig, } from './instrument'; import {NormalCallback, addLeaderAwareRoutingHeader} from './common'; import {protos} from '@google-cloud/spanner-api'; @@ -224,10 +225,6 @@ export interface BatchUpdateCallback { response?: spannerClient.spanner.v1.ExecuteBatchDmlResponse, ): void; } -export interface BatchUpdateOptions { - requestOptions?: Omit; - gaxOptions?: CallOptions; -} export type ReadCallback = NormalCallback; @@ -898,7 +895,13 @@ export class Snapshot extends EventEmitter { maxResumeRetries, requestOptions, columnsMetadata, + keys: _omittedKeys, + ranges: _omittedRanges, + directedReadOptions: rawDirectedReadOptions, + ...cleanRequest } = request; + void _omittedKeys; + void _omittedRanges; const keySet = Snapshot.encodeKeySet(request); const transaction: spannerClient.spanner.v1.ITransactionSelector = {}; @@ -919,23 +922,12 @@ export class Snapshot extends EventEmitter { } const directedReadOptions = this._getDirectedReadOptions( - request.directedReadOptions, + rawDirectedReadOptions, ); - request = Object.assign({}, request); - - delete request.gaxOptions; - delete request.json; - delete request.jsonOptions; - delete request.maxResumeRetries; - delete request.keys; - delete request.ranges; - delete request.requestOptions; - delete request.directedReadOptions; - delete request.columnsMetadata; - const reqOpts: spannerClient.spanner.v1.IReadRequest = Object.assign( - request, + {}, + cleanRequest, { session: this.session.formattedName_!, requestOptions: this.configureTagOptions( @@ -1267,6 +1259,8 @@ export class Snapshot extends EventEmitter { { tableName: table, ...this._traceConfig, + transactionTag: this.requestOptions?.transactionTag, + requestTag: request?.requestOptions?.requestTag, }, span => { this.createReadStream(table, request) @@ -1375,8 +1369,9 @@ export class Snapshot extends EventEmitter { startTrace( 'Snapshot.run', { - ...(query as ExecuteSqlRequest), ...this._traceConfig, + transactionTag: this.requestOptions?.transactionTag, + ...getQueryTraceConfig(query), }, span => { return this.runStream(query) @@ -1510,14 +1505,13 @@ export class Snapshot extends EventEmitter { * ``` */ runStream(query: string | ExecuteSqlRequest): PartialResultStream { - if (typeof query === 'string') { - query = {sql: query} as ExecuteSqlRequest; - } + const originalQuery: ExecuteSqlRequest = + typeof query === 'string' ? {sql: query} : query; - query = Object.assign({}, query) as ExecuteSqlRequest; - query.queryOptions = Object.assign( - Object.assign({}, this.queryOptions), - query.queryOptions, + const queryOptions = Object.assign( + {}, + this.queryOptions, + originalQuery.queryOptions, ); const { @@ -1527,16 +1521,30 @@ export class Snapshot extends EventEmitter { maxResumeRetries, requestOptions, columnsMetadata, - } = query; + types: _omittedTypes, + directedReadOptions: rawDirectedReadOptions, + ...cleanQuery + } = originalQuery; + void _omittedTypes; let reqOpts; const directedReadOptions = this._getDirectedReadOptions( - query.directedReadOptions, + rawDirectedReadOptions, ); + const statementSeqno = this._seqno++; + + let encodedParams: + | { + params: p.IStruct; + paramTypes: {[field: string]: spannerClient.spanner.v1.Type}; + } + | undefined; const sanitizeRequest = () => { - query = query as ExecuteSqlRequest; - const {params, paramTypes} = Snapshot.encodeParams(query); + if (!encodedParams) { + encodedParams = Snapshot.encodeParams(originalQuery); + } + const {params, paramTypes} = encodedParams; const transaction: spannerClient.spanner.v1.ITransactionSelector = {}; if (this.id) { transaction.id = this.id as Uint8Array; @@ -1553,18 +1561,10 @@ export class Snapshot extends EventEmitter { ) { this._setPreviousTransactionId(transaction); } - delete query.gaxOptions; - delete query.json; - delete query.jsonOptions; - delete query.maxResumeRetries; - delete query.requestOptions; - delete query.types; - delete query.directedReadOptions; - delete query.columnsMetadata; - - reqOpts = Object.assign(query, { + reqOpts = Object.assign({}, cleanQuery, { session: this.session.formattedName_!, - seqno: this._seqno++, + seqno: statementSeqno, + queryOptions, requestOptions: this.configureTagOptions( typeof transaction.singleUse !== 'undefined', this.requestOptions?.transactionTag ?? undefined, @@ -1587,10 +1587,9 @@ export class Snapshot extends EventEmitter { } const traceConfig: traceConfig = { - transactionTag: this.requestOptions?.transactionTag, - requestTag: requestOptions?.requestTag, - ...query, ...this._traceConfig, + transactionTag: this.requestOptions?.transactionTag, + ...getQueryTraceConfig(originalQuery), }; return startTrace('Snapshot.runStream', traceConfig, span => { let attempt = 0; @@ -1700,10 +1699,10 @@ export class Snapshot extends EventEmitter { requestOptions = {}, ): IRequestOptions | null { if (!singleUse && transactionTag) { - (requestOptions as IRequestOptions).transactionTag = transactionTag; + return Object.assign({}, requestOptions, {transactionTag}); } - return requestOptions!; + return Object.assign({}, requestOptions); } /** @@ -1716,7 +1715,10 @@ export class Snapshot extends EventEmitter { * @returns {object} */ static encodeKeySet(request: ReadRequest): spannerClient.spanner.v1.IKeySet { - const keySet: spannerClient.spanner.v1.IKeySet = request.keySet || {}; + const keySet: spannerClient.spanner.v1.IKeySet = Object.assign( + {}, + request.keySet, + ); if (request.keys) { keySet.keys = toArray(request.keys as string[]).map( @@ -1802,11 +1804,22 @@ export class Snapshot extends EventEmitter { * @returns {object} */ static encodeParams(request: ExecuteSqlRequest) { - const typeMap = request.types || {}; + if (!request.params && !request.types && !request.paramTypes) { + return { + params: {fields: {}}, + paramTypes: {}, + }; + } + + const enableUuidAsUntyped = + process.env['SPANNER_ENABLE_UUID_AS_UNTYPED'] === 'true'; + const typeMap = Object.assign({}, request.types); - const params: p.IStruct = {fields: request.params?.fields || {}}; + const params: p.IStruct = { + fields: Object.assign({}, request.params?.fields), + }; const paramTypes: {[field: string]: spannerClient.spanner.v1.Type} = - request.paramTypes || {}; + Object.assign({}, request.paramTypes); if (request.params && !request.params.fields) { const fields = {}; @@ -1815,7 +1828,7 @@ export class Snapshot extends EventEmitter { const value = request.params![param]; if (!typeMap[param]) { - typeMap[param] = codec.getType(value); + typeMap[param] = codec.getType(value, enableUuidAsUntyped); } fields[param] = codec.encode(value); }); @@ -1826,7 +1839,7 @@ export class Snapshot extends EventEmitter { if (!isEmpty(typeMap)) { Object.keys(typeMap).forEach(param => { const type = typeMap[param]; - if (process.env['SPANNER_ENABLE_UUID_AS_UNTYPED'] === 'true') { + if (enableUuidAsUntyped) { const typeObject = codec.createTypeObject(type); if ( (type.child && @@ -2001,10 +2014,9 @@ export class Dml extends Snapshot { return startTrace( 'Dml.runUpdate', { - ...query, ...this._traceConfig, transactionTag: this.requestOptions?.transactionTag, - requestTag: query.requestOptions?.requestTag, + ...getQueryTraceConfig(query), }, span => { this.run( @@ -3535,8 +3547,8 @@ export class PartitionedDml extends Dml { return startTrace( 'PartitionedDml.runUpdate', { - ...(query as ExecuteSqlRequest), ...this._traceConfig, + ...getQueryTraceConfig(query), }, span => { super.runUpdate(query, (err, count) => { diff --git a/handwritten/spanner/test/batch-transaction.ts b/handwritten/spanner/test/batch-transaction.ts index b4f18298b684..990bee6bb8bd 100644 --- a/handwritten/spanner/test/batch-transaction.ts +++ b/handwritten/spanner/test/batch-transaction.ts @@ -215,6 +215,25 @@ describe('BatchTransaction', () => { assert.strictEqual(gaxOpts, undefined); }); + it('should not mutate the original query object', () => { + const query = { + sql: 'SELECT * FROM Singers', + gaxOptions: GAX_OPTS as CallOptions, + params: {}, + types: {}, + dataBoostEnabled: true, + }; + const queryCopy = Object.assign({}, query); + sandbox.stub(batchTransaction, 'createPartitions_'); + (sandbox.stub(FakeTransaction, 'encodeParams') as sinon.SinonStub) + .withArgs(query) + .returns({}); + + batchTransaction.createQueryPartitions(query, assert.ifError); + + assert.deepStrictEqual(query, queryCopy); + }); + it('should make the correct request using await', async () => { const fakeParams = { params: {a: 'b'}, @@ -346,6 +365,21 @@ describe('BatchTransaction', () => { done(); }); }); + + it('should not mutate config.reqOpts and should omit partitionOptions from returned partitions', done => { + const partitionOptions = {maxPartitions: 10}; + const reqOpts = {sql: 'SELECT 1', partitionOptions}; + const config = {reqOpts}; + + batchTransaction.createPartitions_(config, (err, parts) => { + assert.ifError(err); + assert.strictEqual(reqOpts.partitionOptions, partitionOptions); + parts.forEach(part => { + assert.strictEqual('partitionOptions' in part, false); + }); + done(); + }); + }); }); describe('createReadPartitions', () => { @@ -428,6 +462,25 @@ describe('BatchTransaction', () => { Object.assign({[LEADER_AWARE_ROUTING_HEADER]: 'true'}), ); }); + + it('should not mutate the original options object', () => { + const options = { + table: 'abc', + keys: ['a', 'b'], + ranges: [{}, {}], + gaxOptions: GAX_OPTS, + dataBoostEnabled: true, + }; + const optionsCopy = Object.assign({}, options); + sandbox.stub(batchTransaction, 'createPartitions_'); + (sandbox.stub(FakeTransaction, 'encodeKeySet') as sinon.SinonStub) + .withArgs(options) + .returns({}); + + batchTransaction.createReadPartitions(options, assert.ifError); + + assert.deepStrictEqual(options, optionsCopy); + }); }); describe('execute', () => { diff --git a/handwritten/spanner/test/instrument.ts b/handwritten/spanner/test/instrument.ts new file mode 100644 index 000000000000..edae406a9c23 --- /dev/null +++ b/handwritten/spanner/test/instrument.ts @@ -0,0 +1,90 @@ +/*! + * Copyright 2026 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 + * + * http://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, it} from 'mocha'; +import {getQueryTraceConfig} from '../src/instrument'; + +describe('instrument', () => { + describe('getQueryTraceConfig', () => { + it('should return empty object for null or undefined', () => { + assert.deepStrictEqual(getQueryTraceConfig(null), {}); + assert.deepStrictEqual(getQueryTraceConfig(undefined), {}); + assert.deepStrictEqual(getQueryTraceConfig(), {}); + }); + + it('should extract sql string without spreading characters', () => { + const sql = 'SELECT 1'; + const result = getQueryTraceConfig(sql); + + assert.deepStrictEqual(result, {sql}); + assert.strictEqual((result as any)['0'], undefined); + }); + + it('should extract sql and requestTag from object query', () => { + const query = { + sql: 'SELECT * FROM Singers WHERE SingerId = @id', + requestOptions: { + requestTag: 'find-singers', + transactionTag: 'read-tx', + }, + }; + + const result = getQueryTraceConfig(query); + + assert.deepStrictEqual(result, { + sql: query.sql, + requestTag: 'find-singers', + }); + }); + + it('should handle object query without requestOptions', () => { + const query = { + sql: 'SELECT 1', + }; + + const result = getQueryTraceConfig(query); + + assert.deepStrictEqual(result, { + sql: 'SELECT 1', + requestTag: undefined, + }); + }); + + it('should handle object query with empty requestOptions', () => { + const query = { + sql: 'SELECT 1', + requestOptions: {}, + }; + + const result = getQueryTraceConfig(query); + + assert.deepStrictEqual(result, { + sql: 'SELECT 1', + requestTag: undefined, + }); + }); + + it('should handle empty object query', () => { + const result = getQueryTraceConfig({}); + + assert.deepStrictEqual(result, { + sql: undefined, + requestTag: undefined, + }); + }); + }); +}); diff --git a/handwritten/spanner/test/transaction.ts b/handwritten/spanner/test/transaction.ts index 1e9794de5f86..72214c838173 100644 --- a/handwritten/spanner/test/transaction.ts +++ b/handwritten/spanner/test/transaction.ts @@ -339,6 +339,7 @@ describe('Transaction', () => { beforeEach(() => { PARTIAL_RESULT_STREAM.callsFake(makeRequest => makeRequest()); + REQUEST_STREAM.callsFake(() => new EventEmitter()); }); it('should send the correct request', () => { @@ -389,6 +390,26 @@ describe('Transaction', () => { assert.deepStrictEqual(reqOpts.requestOptions, {requestTag}); }); + it('should not mutate the original request object', () => { + const request = { + keys: ['1'], + ranges: [{}, {}], + columns: ['SingerId'], + json: true, + jsonOptions: {}, + gaxOptions: {}, + maxResumeRetries: 5, + requestOptions: {requestTag: 'tag'}, + directedReadOptions: {}, + columnsMetadata: {}, + }; + const requestCopy = Object.assign({}, request); + + snapshot.createReadStream(TABLE, request); + + assert.deepStrictEqual(request, requestCopy); + }); + it('should send the correct `reqOpts`', () => { const id = 'transaction-id-123'; const fakeKeySet = {all: true}; @@ -726,6 +747,7 @@ describe('Transaction', () => { beforeEach(() => { PARTIAL_RESULT_STREAM.callsFake(makeRequest => makeRequest()); + REQUEST_STREAM.callsFake(() => new EventEmitter()); }); it('should send the correct request', () => { @@ -765,6 +787,68 @@ describe('Transaction', () => { assert.deepStrictEqual(reqOpts.transaction, expectedTransaction); }); + it('should not mutate the original query object', () => { + const query = { + sql: 'SELECT * FROM `MyTable` WHERE id = @id', + json: true, + jsonOptions: {}, + gaxOptions: {}, + maxResumeRetries: 5, + params: {id: '1'}, + types: {}, + requestOptions: {requestTag: 'foo'}, + columnsMetadata: {}, + }; + const queryCopy = { + sql: query.sql, + json: true, + jsonOptions: {}, + gaxOptions: {}, + maxResumeRetries: 5, + params: Object.assign({}, query.params), + types: Object.assign({}, query.types), + requestOptions: Object.assign({}, query.requestOptions), + columnsMetadata: {}, + }; + + snapshot.requestOptions = {transactionTag: 'tx-tag'}; + snapshot.runStream(query); + + assert.deepStrictEqual(query, queryCopy); + }); + + it('should preserve parameters and metadata on multiple makeRequest calls', () => { + const encodeParamsSpy = sandbox.spy(Snapshot, 'encodeParams'); + const query = { + sql: 'SELECT * FROM `MyTable` WHERE id = @id', + params: {id: '1'}, + types: {id: 'string'}, + requestOptions: {requestTag: 'custom-tag'}, + }; + snapshot.runStream(query); + + const makeRequest = PARTIAL_RESULT_STREAM.lastCall.args[0]; + makeRequest(); + const firstCallReqOpts = REQUEST_STREAM.lastCall.args[0].reqOpts; + + makeRequest('resume-token'); + const secondCallReqOpts = REQUEST_STREAM.lastCall.args[0].reqOpts; + + assert.strictEqual(encodeParamsSpy.callCount, 1); + assert.deepStrictEqual( + firstCallReqOpts.params, + secondCallReqOpts.params, + ); + assert.deepStrictEqual( + firstCallReqOpts.paramTypes, + secondCallReqOpts.paramTypes, + ); + assert.deepStrictEqual( + firstCallReqOpts.requestOptions, + secondCallReqOpts.requestOptions, + ); + }); + it('should set request tag', () => { const requestTag = 'foo'; const query = Object.assign({}, QUERY, { @@ -847,6 +931,22 @@ describe('Transaction', () => { assert.strictEqual(call2.reqOpts.seqno, 2); }); + it('should preserve the same `seqno` across makeRequest retries and resumptions', () => { + snapshot.runStream(QUERY); + + const makeRequest = PARTIAL_RESULT_STREAM.lastCall.args[0]; + makeRequest(); + const call1 = REQUEST_STREAM.lastCall.args[0]; + + // Simulate transaction ID arrival and stream resumption + snapshot.id = 'tx-123'; + makeRequest('resume-token'); + const call2 = REQUEST_STREAM.lastCall.args[0]; + + assert.strictEqual(call1.reqOpts.seqno, 1); + assert.strictEqual(call2.reqOpts.seqno, 1); + }); + it('should pass a stream to `PartialResultStream`', () => { const fakeStream = new EventEmitter(); @@ -1109,6 +1209,14 @@ describe('Transaction', () => { assert.deepStrictEqual(keySet, fakeKeySet); }); + + it('should not mutate the original keySet object', () => { + const fakeKeySet = {keys: []}; + const fakeKeySetCopy = Object.assign({}, fakeKeySet); + Snapshot.encodeKeySet({keySet: fakeKeySet, keys: ['a']}); + + assert.deepStrictEqual(fakeKeySet, fakeKeySetCopy); + }); }); describe('encodeTimestampBounds', () => { @@ -1274,6 +1382,80 @@ describe('Transaction', () => { assert.strictEqual(paramTypes.a, expectedTypes.a); }); + + it('should not mutate the original types or paramTypes objects', () => { + const fakeParams = {a: 'foo', b: 3}; + const fakeTypes = {b: 'number'}; + const fakeParamTypes = {}; + + Snapshot.encodeParams({ + params: fakeParams, + types: fakeTypes, + paramTypes: fakeParamTypes, + }); + + assert.deepStrictEqual(fakeTypes, {b: 'number'}); + assert.deepStrictEqual(fakeParamTypes, {}); + }); + + it('should return empty params and paramTypes for parameterless queries', () => { + const result = Snapshot.encodeParams({}); + assert.deepStrictEqual(result, { + params: {fields: {}}, + paramTypes: {}, + }); + }); + + it('should omit TYPE_CODE_UNSPECIFIED when SPANNER_ENABLE_UUID_AS_UNTYPED is true', () => { + const savedEnv = process.env['SPANNER_ENABLE_UUID_AS_UNTYPED']; + try { + process.env['SPANNER_ENABLE_UUID_AS_UNTYPED'] = 'true'; + const fakeParams = {a: 'some-uuid'}; + sandbox.stub(codec, 'getType').returns({type: 'unspecified'}); + sandbox.stub(codec, 'createTypeObject').returns({ + code: 'TYPE_CODE_UNSPECIFIED', + } as any); + + const {paramTypes} = Snapshot.encodeParams({params: fakeParams}); + assert.strictEqual(paramTypes.a, undefined); + } finally { + if (savedEnv === undefined) { + delete process.env['SPANNER_ENABLE_UUID_AS_UNTYPED']; + } else { + process.env['SPANNER_ENABLE_UUID_AS_UNTYPED'] = savedEnv; + } + } + }); + }); + + describe('configureTagOptions', () => { + it('should not mutate the original requestOptions object', () => { + const originalRequestOptions = {requestTag: 'tag-1'}; + const copy = Object.assign({}, originalRequestOptions); + const configured = snapshot.configureTagOptions( + false, + 'tx-tag', + originalRequestOptions, + ); + + assert.deepStrictEqual(originalRequestOptions, copy); + assert.deepStrictEqual(configured, { + requestTag: 'tag-1', + transactionTag: 'tx-tag', + }); + }); + + it('should return a new object when singleUse is true', () => { + const originalRequestOptions = {requestTag: 'tag-1'}; + const configured = snapshot.configureTagOptions( + true, + 'tx-tag', + originalRequestOptions, + ); + + assert.notStrictEqual(configured, originalRequestOptions); + assert.deepStrictEqual(configured, originalRequestOptions); + }); }); });