diff --git a/handwritten/spanner/observability-test/spanner.ts b/handwritten/spanner/observability-test/spanner.ts index 860c1d806518..a9bb614160f8 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 { @@ -1778,6 +1838,7 @@ describe('Traces for ExecuteStream broken stream retries', () => { ); done(); + return null; }) .catch(err => done(err)); }); @@ -1938,9 +1999,7 @@ describe('Traces for ExecuteStream broken stream retries', () => { assert.strictEqual(attempts, 1); tx! .commit() - .then(() => { - database.close().catch(assert.ifError); - }) + .then(() => database.close()) .catch(assert.ifError); }); }); 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/database.ts b/handwritten/spanner/src/database.ts index 1eee2d4b8add..516f4ee7b5d8 100644 --- a/handwritten/spanner/src/database.ts +++ b/handwritten/spanner/src/database.ts @@ -118,6 +118,7 @@ import { setSpanError, setSpanErrorAndException, traceConfig, + getQueryTraceConfig, } from './instrument'; import { AtomicCounter, @@ -2944,8 +2945,8 @@ class Database extends common.GrpcServiceObject { startTrace( 'Database.run', { - ...(query as ExecuteSqlRequest), ...this._traceConfig, + ...getQueryTraceConfig(query), }, span => { this.runStream(query, options) @@ -2985,9 +2986,9 @@ class Database extends common.GrpcServiceObject { options: TimestampBounds, callback: RunCallback, ): void { - const traceConfig = { - ...(query as ExecuteSqlRequest), + const traceConfig: traceConfig = { ...this._traceConfig, + ...getQueryTraceConfig(query), }; startTrace('Database.run', traceConfig, runSpan => { @@ -3142,10 +3143,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) => { @@ -3335,9 +3334,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 a96e1ff289a5..5b9e4c8e0727 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 b440e7a17ac3..07348ee19c05 100644 --- a/handwritten/spanner/src/transaction.ts +++ b/handwritten/spanner/src/transaction.ts @@ -40,6 +40,7 @@ import { setSpanError, setSpanErrorAndException, traceConfig, + getQueryTraceConfig, } from './instrument'; import {NormalCallback, addLeaderAwareRoutingHeader} from './common'; import {protos} from '@google-cloud/spanner-api'; @@ -931,7 +932,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 = {}; @@ -952,23 +959,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( @@ -1301,6 +1297,8 @@ export class Snapshot extends EventEmitter { { tableName: table, ...this._traceConfig, + transactionTag: this.requestOptions?.transactionTag, + requestTag: request?.requestOptions?.requestTag, }, span => { this.createReadStream(table, request) @@ -1426,8 +1424,9 @@ export class Snapshot extends EventEmitter { startTrace( 'Snapshot.run', { - ...(query as ExecuteSqlRequest), ...this._traceConfig, + transactionTag: this.requestOptions?.transactionTag, + ...getQueryTraceConfig(query), }, span => { this.runStream(query) @@ -1543,11 +1542,10 @@ export class Snapshot extends EventEmitter { addLeaderAwareRoutingHeader(headers); } - const traceConfig = { - transactionTag: this.requestOptions?.transactionTag, - requestTag: requestOptions?.requestTag, - ...query, + const traceConfig: traceConfig = { ...this._traceConfig, + transactionTag: this.requestOptions?.transactionTag, + ...getQueryTraceConfig(queryInput), }; const executeWithSpans = ( @@ -1925,14 +1923,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 { @@ -1942,16 +1939,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; @@ -1968,18 +1979,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, @@ -2002,10 +2005,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; @@ -2115,10 +2117,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); } /** @@ -2131,7 +2133,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( @@ -2217,12 +2222,21 @@ export class Snapshot extends EventEmitter { * @returns {object} */ static encodeParams(request: ExecuteSqlRequest) { + if (!request.params && !request.types && !request.paramTypes) { + return { + params: {fields: {}}, + paramTypes: {}, + }; + } + const isUuidUntyped = codec.isUuidUntypedEnv(); - const typeMap = request.types || {}; + 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 = {}; @@ -2424,10 +2438,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( @@ -3958,8 +3971,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 647f6c763d8e..ec3376fec3b3 100644 --- a/handwritten/spanner/test/transaction.ts +++ b/handwritten/spanner/test/transaction.ts @@ -393,6 +393,7 @@ describe('Transaction', () => { beforeEach(() => { PARTIAL_RESULT_STREAM.callsFake(makeRequest => makeRequest()); + REQUEST_STREAM.callsFake(() => new EventEmitter()); }); it('should send the correct request', () => { @@ -443,6 +444,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}; @@ -1579,6 +1600,7 @@ describe('Transaction', () => { beforeEach(() => { PARTIAL_RESULT_STREAM.callsFake(makeRequest => makeRequest()); + REQUEST_STREAM.callsFake(() => new EventEmitter()); }); it('should send the correct request', () => { @@ -1618,6 +1640,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, { @@ -1700,6 +1784,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(); @@ -1962,6 +2062,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', () => { @@ -2127,6 +2235,81 @@ 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']; + sandbox.stub(process, 'emitWarning'); + 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); + }); }); });