From f2eae740fc930aa952fba68b3df91db732e12e32 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Wed, 7 Oct 2026 20:33:26 +0000 Subject: [PATCH 1/8] refactor(firestore): align DML and literals stage overloads with pipeline API conventions Follow the existing Pipeline stage convention of `stage(requiredArg)` / variadic convenience overloads plus a single `stage(options)` overload, where the options object carries the required params. - literals (PipelineSource and Pipeline): remove the `(documents, options?)` overload; `documents` is now required in LiteralsStageOptions. - update: remove the `Record | Map` overload (not in the API proposal, no precedent, and indistinguishable at runtime from an options object); add `update(options: UpdateStageOptions)` with `transformedFields`. - delete: add `delete(options: DeleteStageOptions)` so rawOptions can be provided. - upsert: remove the `(additionalFields[], options?)` overload; use `upsert({additionalFields, ...})` instead. --- .../firestore/dev/src/pipelines/pipelines.ts | 199 ++++++++---------- .../firestore/dev/src/pipelines/stage.ts | 89 ++++---- .../firestore/dev/system-test/pipeline.ts | 12 +- .../firestore/dev/test/pipelines/pipeline.ts | 40 +++- .../firestore/dev/test/pipelines/stage.ts | 60 ++++++ handwritten/firestore/types/firestore.d.ts | 70 +++--- 6 files changed, 270 insertions(+), 200 deletions(-) diff --git a/handwritten/firestore/dev/src/pipelines/pipelines.ts b/handwritten/firestore/dev/src/pipelines/pipelines.ts index 9eaeb384970e..75dcb470f783 100644 --- a/handwritten/firestore/dev/src/pipelines/pipelines.ts +++ b/handwritten/firestore/dev/src/pipelines/pipelines.ts @@ -107,6 +107,7 @@ import { InternalDefineStageOptions, InternalSubcollectionStageOptions, UpdateStage, + InternalUpdateStageOptions, Search, InternalSearchStageOptions, InsertStage, @@ -125,6 +126,21 @@ import { ReflectionObject, } from 'protobufjs'; +/** + * Normalizes the arguments of the `literals(documents)` and + * `literals(options)` overloads into a single options object. + * @private + * @internal + */ +function toLiteralsStageOptions( + documentsOrOptions: + Array> | firestore.Pipelines.LiteralsStageOptions, +): InternalLiteralsStageOptions { + return Array.isArray(documentsOrOptions) + ? {documents: documentsOrOptions} + : documentsOrOptions; +} + /** * Represents the source of a Firestore `Pipeline`. */ @@ -135,37 +151,21 @@ export class PipelineSource implements firestore.Pipelines.PipelineSource { * Set the pipeline's source to the in-memory documents specified by the given records. * * @param documents An array of objects/records specifying the in-memory documents. + */ + literals(documents: Array>): Pipeline; + /** + * Set the pipeline's source to the in-memory documents specified by the given records. + * * @param options Options defining how this LiteralsSource stage is evaluated. */ - literals( - documents: Array>, - options?: firestore.Pipelines.LiteralsStageOptions, - ): Pipeline; literals(options: firestore.Pipelines.LiteralsStageOptions): Pipeline; literals( - docsOrOptions: + documentsOrOptions: Array> | firestore.Pipelines.LiteralsStageOptions, - options?: firestore.Pipelines.LiteralsStageOptions, ): Pipeline { - let documents: Array> = []; - let opts: InternalLiteralsStageOptions = {}; - - if (Array.isArray(docsOrOptions)) { - documents = docsOrOptions; - opts = options ?? {}; - } else if ( - docsOrOptions && - Array.isArray( - (docsOrOptions as firestore.Pipelines.LiteralsStageOptions).documents, - ) - ) { - const {documents: docs, ...rest} = - docsOrOptions as firestore.Pipelines.LiteralsStageOptions; - documents = docs ?? []; - opts = rest; - } - - return new Pipeline(this.db, [new LiteralsSource(documents, opts)]); + return new Pipeline(this.db, [ + new LiteralsSource(toLiteralsStageOptions(documentsOrOptions)), + ]); } /** @@ -1788,8 +1788,16 @@ export class Pipeline implements firestore.Pipelines.Pipeline { * * @return A new {@code Pipeline} object with this stage appended to the stage list. */ - delete(): Pipeline { - return this._addStage(new DeleteStage()); + delete(): Pipeline; + /** + * Performs a delete operation on documents from previous stages. + * + * @param options - Options defining how this Delete stage is evaluated. + * @return A new {@code Pipeline} object with this stage appended to the stage list. + */ + delete(options: firestore.Pipelines.DeleteStageOptions): Pipeline; + delete(options?: firestore.Pipelines.DeleteStageOptions): Pipeline { + return this._addStage(new DeleteStage(options ?? {})); } /** @@ -1819,39 +1827,40 @@ export class Pipeline implements firestore.Pipelines.Pipeline { /** * Performs an update operation using documents from previous stages. * - * @param fieldsMap - Map of field transformations to apply. + * @param options - Options defining how this Update stage is evaluated. * @return A new {@code Pipeline} object with this stage appended to the stage list. */ + update(options: firestore.Pipelines.UpdateStageOptions): Pipeline; update( - fieldsMap: Record | Map, - ): Pipeline; - update( - transformedFieldsOrMap?: + transformedFieldOrOptions?: | AliasedExpression | AliasedExpression[] - | Map - | Record, + | firestore.Pipelines.UpdateStageOptions, ...additionalFields: AliasedExpression[] ): Pipeline { - if (isAliasedExpr(transformedFieldsOrMap)) { - return this._addStage( - new UpdateStage([transformedFieldsOrMap, ...additionalFields]), - ); - } else if ( - transformedFieldsOrMap instanceof Map || - Array.isArray(transformedFieldsOrMap) - ) { - return this._addStage(new UpdateStage(transformedFieldsOrMap)); - } else if ( - transformedFieldsOrMap && - isPlainObject(transformedFieldsOrMap) - ) { - const map = new Map( - Object.entries(transformedFieldsOrMap as Record), - ); - return this._addStage(new UpdateStage(map)); + let options: firestore.Pipelines.UpdateStageOptions; + if (isAliasedExpr(transformedFieldOrOptions)) { + options = { + transformedFields: [transformedFieldOrOptions, ...additionalFields], + }; + } else if (Array.isArray(transformedFieldOrOptions)) { + options = {transformedFields: transformedFieldOrOptions}; + } else { + options = transformedFieldOrOptions ?? {}; } - return this._addStage(new UpdateStage()); + + const {transformedFields, ...rest} = options; + const internalOptions: InternalUpdateStageOptions = { + ...rest, + ...(transformedFields + ? { + transformedFields: selectablesToMap( + transformedFields as AliasedExpression[], + ), + } + : {}), + }; + return this._addStage(new UpdateStage(internalOptions)); } /** @@ -1872,13 +1881,6 @@ export class Pipeline implements firestore.Pipelines.Pipeline { * @return A new {@code Pipeline} object with this stage appended to the stage list. */ upsert(): Pipeline; - /** - * Performs an upsert operation on documents from previous stages. - * - * @param options - Options defining how this Upsert stage is evaluated. - * @return A new {@code Pipeline} object with this stage appended to the stage list. - */ - upsert(options: firestore.Pipelines.UpsertStageOptions): Pipeline; /** * Performs an upsert operation on documents from previous stages. * @@ -1893,83 +1895,46 @@ export class Pipeline implements firestore.Pipelines.Pipeline { /** * Performs an upsert operation on documents from previous stages. * - * @param additionalFields - Additional fields to apply on upsert. * @param options - Options defining how this Upsert stage is evaluated. * @return A new {@code Pipeline} object with this stage appended to the stage list. */ + upsert(options: firestore.Pipelines.UpsertStageOptions): Pipeline; upsert( - additionalFields: AliasedExpression[], - options?: firestore.Pipelines.UpsertStageOptions, - ): Pipeline; - upsert( - additionalFieldsOrOptions?: - | AliasedExpression - | AliasedExpression[] - | firestore.Pipelines.UpsertStageOptions, - ...restArgs: unknown[] + additionalFieldOrOptions?: + AliasedExpression | firestore.Pipelines.UpsertStageOptions, + ...additionalFields: AliasedExpression[] ): Pipeline { - let additionalFields: AliasedExpression[] | undefined; - let options: firestore.Pipelines.UpsertStageOptions | undefined; - - if (isAliasedExpr(additionalFieldsOrOptions)) { - additionalFields = [ - additionalFieldsOrOptions, - ...(restArgs as AliasedExpression[]), - ]; - options = undefined; - } else if (Array.isArray(additionalFieldsOrOptions)) { - additionalFields = additionalFieldsOrOptions; - options = restArgs[0] as - firestore.Pipelines.UpsertStageOptions | undefined; - } else { - additionalFields = undefined; - options = additionalFieldsOrOptions; + if (isAliasedExpr(additionalFieldOrOptions)) { + return this._addStage( + new UpsertStage({ + additionalFields: [additionalFieldOrOptions, ...additionalFields], + }), + ); } - - return this._addStage( - new UpsertStage({ - ...options, - ...(additionalFields ? {additionalFields} : {}), - }), - ); + return this._addStage(new UpsertStage(additionalFieldOrOptions ?? {})); } /** * Appends a literals stage to the pipeline. * * @param documents An array of objects/records specifying in-memory documents. + * @return A new {@code Pipeline} object with this stage appended to the stage list. + */ + literals(documents: Array>): Pipeline; + /** + * Appends a literals stage to the pipeline. + * * @param options Options defining how this LiteralsSource stage is evaluated. * @return A new {@code Pipeline} object with this stage appended to the stage list. */ - literals( - documents: Array>, - options?: firestore.Pipelines.LiteralsStageOptions, - ): Pipeline; literals(options: firestore.Pipelines.LiteralsStageOptions): Pipeline; literals( - docsOrOptions: + documentsOrOptions: Array> | firestore.Pipelines.LiteralsStageOptions, - options?: firestore.Pipelines.LiteralsStageOptions, ): Pipeline { - let documents: Array> = []; - let opts: InternalLiteralsStageOptions = {}; - - if (Array.isArray(docsOrOptions)) { - documents = docsOrOptions; - opts = options ?? {}; - } else if ( - docsOrOptions && - Array.isArray( - (docsOrOptions as firestore.Pipelines.LiteralsStageOptions).documents, - ) - ) { - const {documents: docs, ...rest} = - docsOrOptions as firestore.Pipelines.LiteralsStageOptions; - documents = docs ?? []; - opts = rest; - } - - return this._addStage(new LiteralsSource(documents, opts)); + return this._addStage( + new LiteralsSource(toLiteralsStageOptions(documentsOrOptions)), + ); } /** diff --git a/handwritten/firestore/dev/src/pipelines/stage.ts b/handwritten/firestore/dev/src/pipelines/stage.ts index 4404c5d55f6e..3c3cb369c502 100644 --- a/handwritten/firestore/dev/src/pipelines/stage.ts +++ b/handwritten/firestore/dev/src/pipelines/stage.ts @@ -907,6 +907,11 @@ export class RawStage implements Stage { } } +/** + * Internal options for Delete stage. + */ +export type InternalDeleteStageOptions = firestore.Pipelines.DeleteStageOptions; + /** * Delete stage. */ @@ -914,7 +919,7 @@ export class DeleteStage implements Stage { name = 'delete'; readonly optionsUtil = new OptionsUtil({}); - constructor() {} + constructor(private options: InternalDeleteStageOptions = {}) {} _toProto(serializer: Serializer): api.Pipeline.IStage { const args: api.IValue[] = []; @@ -922,50 +927,58 @@ export class DeleteStage implements Stage { return { name: this.name, args, - options: this.optionsUtil.getOptionsProto(serializer, {}, {}), + options: this.optionsUtil.getOptionsProto( + serializer, + this.options, + this.options.rawOptions, + ), }; } _validateUserData(_: boolean): void {} } +/** + * Internal options for Update stage. + */ +export type InternalUpdateStageOptions = Omit< + firestore.Pipelines.UpdateStageOptions, + 'transformedFields' +> & { + transformedFields?: Map; +}; + /** * Update stage. */ export class UpdateStage implements Stage { name = 'update'; readonly optionsUtil = new OptionsUtil({}); - private readonly transformedMap?: Map; - constructor( - transformedFields?: AliasedExpression[] | Map, - ) { - if (transformedFields instanceof Map) { - this.transformedMap = transformedFields; - } else if (transformedFields && transformedFields.length > 0) { - this.transformedMap = selectablesToMap(transformedFields); - } - } + constructor(private options: InternalUpdateStageOptions = {}) {} _toProto(serializer: Serializer): api.Pipeline.IStage { - const args: api.IValue[] = []; - - if (this.transformedMap && this.transformedMap.size > 0) { - args.push(serializer.encodeValue(this.transformedMap)!); - } else { - args.push(serializer.encodeValue(new Map())!); - } + const args: api.IValue[] = [ + serializer.encodeValue(this.options.transformedFields ?? new Map())!, + ]; return { name: this.name, args, - options: this.optionsUtil.getOptionsProto(serializer, {}, {}), + options: this.optionsUtil.getOptionsProto( + serializer, + this.options, + this.options.rawOptions, + ), }; } _validateUserData(ignoreUndefinedProperties: boolean): void { - if (this.transformedMap) { - validateUserDataHelper(this.transformedMap, ignoreUndefinedProperties); + if (this.options.transformedFields) { + validateUserDataHelper( + this.options.transformedFields, + ignoreUndefinedProperties, + ); } } } @@ -1061,29 +1074,12 @@ export class UpsertStage implements Stage { private readonly documentIdExpr?: Expression; private readonly additionalFields: Map; - private readonly options: InternalUpsertStageOptions; - - constructor( - additionalFieldsOrOptions?: - AliasedExpression[] | InternalUpsertStageOptions, - options?: InternalUpsertStageOptions, - ) { - let fields: AliasedExpression[] = []; - let opts: InternalUpsertStageOptions = {}; - - if (Array.isArray(additionalFieldsOrOptions)) { - fields = additionalFieldsOrOptions; - opts = options ?? {}; - } else if (additionalFieldsOrOptions) { - opts = additionalFieldsOrOptions; - } else if (options) { - opts = options; - } - this.options = opts; + constructor(private readonly options: InternalUpsertStageOptions = {}) { + const opts = options; const resolvedFields = (opts.additionalFields ?? opts.transforms ?? - fields) as AliasedExpression[]; + []) as AliasedExpression[]; this.additionalFields = selectablesToMap(resolvedFields); if (opts.collection) { this.collectionPath = @@ -1147,13 +1143,10 @@ export class LiteralsSource implements Stage { name = 'literals'; readonly optionsUtil = new OptionsUtil({}); - constructor( - readonly documents: Array>, - private options: InternalLiteralsStageOptions = {}, - ) {} + constructor(private options: InternalLiteralsStageOptions) {} _toProto(serializer: Serializer): api.Pipeline.IStage { - const args: api.IValue[] = this.documents.map(doc => + const args: api.IValue[] = this.options.documents.map(doc => serializer.encodeValue(doc)!, ); return { @@ -1168,7 +1161,7 @@ export class LiteralsSource implements Stage { } _validateUserData(ignoreUndefinedProperties: boolean): void { - this.documents.forEach(doc => { + this.options.documents.forEach(doc => { Object.values(doc).forEach(val => { if (hasUserData(val)) { val._validateUserData(ignoreUndefinedProperties); diff --git a/handwritten/firestore/dev/system-test/pipeline.ts b/handwritten/firestore/dev/system-test/pipeline.ts index 13ebb045a01c..dcc3324dd21b 100644 --- a/handwritten/firestore/dev/system-test/pipeline.ts +++ b/handwritten/firestore/dev/system-test/pipeline.ts @@ -717,15 +717,17 @@ describe.skipClassic('Pipeline class', () => { expect(docSnap.get('rating')).to.equal(4.7); }); - it('can upsert (update) an existing document with array of expressions', async () => { + it('can upsert (update) an existing document with additionalFields option', async () => { const res = await firestore .pipeline() .collection(dmlCol.path) .where(equal(field('__name__').documentId(), 'book1')) - .upsert([ - constant('Updated Genre List').as('genre'), - constant(5.0).as('rating'), - ]) + .upsert({ + additionalFields: [ + constant('Updated Genre List').as('genre'), + constant(5.0).as('rating'), + ], + }) .execute({atomic: true}); expectResults(res, {documents_modified: 1}); diff --git a/handwritten/firestore/dev/test/pipelines/pipeline.ts b/handwritten/firestore/dev/test/pipelines/pipeline.ts index 51269f4e6bf9..3967083f0d7a 100644 --- a/handwritten/firestore/dev/test/pipelines/pipeline.ts +++ b/handwritten/firestore/dev/test/pipelines/pipeline.ts @@ -556,6 +556,35 @@ describe('execute(Pipeline|PipelineExecuteOptions)', () => { }, ]); }); + + it('serializes options update({transformedFields})', async () => { + const spy = sinon.fake.returns(stream()); + const firestore = await createInstance({ + executePipeline: spy, + }); + + await firestore + .pipeline() + .collection('foo') + .update({transformedFields: [constant(true).as('active')]}) + .execute(); + + const stages = + spy.args[FIRST_CALL][EXECUTE_PIPELINE_REQUEST]['structuredPipeline'][ + 'pipeline' + ]['stages']; + + expect(stages[1].name).to.equal('update'); + expect(stages[1].args).to.deep.equal([ + { + mapValue: { + fields: { + active: {booleanValue: true}, + }, + }, + }, + ]); + }); }); describe('upsert stage overloads', () => { @@ -678,7 +707,7 @@ describe('execute(Pipeline|PipelineExecuteOptions)', () => { expect(stages[1].options).to.be.undefined; }); - it('serializes array of expressions without options upsert([expr])', async () => { + it('serializes additionalFields option upsert({additionalFields})', async () => { const spy = sinon.fake.returns(stream()); const firestore = await createInstance({ executePipeline: spy, @@ -687,7 +716,7 @@ describe('execute(Pipeline|PipelineExecuteOptions)', () => { await firestore .pipeline() .collection('foo') - .upsert([constant(true).as('active')]) + .upsert({additionalFields: [constant(true).as('active')]}) .execute(); const stages = @@ -708,7 +737,7 @@ describe('execute(Pipeline|PipelineExecuteOptions)', () => { expect(stages[1].options).to.be.undefined; }); - it('serializes array of expressions with options upsert([expr], options)', async () => { + it('serializes additionalFields with other options upsert({additionalFields, collection})', async () => { const spy = sinon.fake.returns(stream()); const firestore = await createInstance({ executePipeline: spy, @@ -717,7 +746,10 @@ describe('execute(Pipeline|PipelineExecuteOptions)', () => { await firestore .pipeline() .collection('foo') - .upsert([constant(true).as('active')], {collection: 'users'}) + .upsert({ + additionalFields: [constant(true).as('active')], + collection: 'users', + }) .execute(); const stages = diff --git a/handwritten/firestore/dev/test/pipelines/stage.ts b/handwritten/firestore/dev/test/pipelines/stage.ts index e1fd454e7402..b138f89e8991 100644 --- a/handwritten/firestore/dev/test/pipelines/stage.ts +++ b/handwritten/firestore/dev/test/pipelines/stage.ts @@ -249,6 +249,66 @@ describe('stage option serialization', () => { }), stageIndex: 1, }, + { + name: 'literals source stage', + pipeline: firestore => + firestore.pipeline().literals({ + documents: [{foo: 'bar'}], + rawOptions, + }), + }, + { + name: 'literals stage', + pipeline: firestore => + firestore + .pipeline() + .database() + .literals({ + documents: [{foo: 'bar'}], + rawOptions, + }), + stageIndex: 1, + }, + { + name: 'delete stage', + pipeline: firestore => + firestore.pipeline().database().delete({ + rawOptions, + }), + stageIndex: 1, + }, + { + name: 'update stage', + pipeline: firestore => + firestore + .pipeline() + .database() + .update({ + transformedFields: [constant(true).as('foo')], + rawOptions, + }), + stageIndex: 1, + }, + { + name: 'insert stage', + pipeline: firestore => + firestore.pipeline().database().insert({ + rawOptions, + }), + stageIndex: 1, + }, + { + name: 'upsert stage', + pipeline: firestore => + firestore + .pipeline() + .database() + .upsert({ + additionalFields: [constant(true).as('foo')], + rawOptions, + }), + stageIndex: 1, + }, { name: 'search stage', pipeline: firestore => diff --git a/handwritten/firestore/types/firestore.d.ts b/handwritten/firestore/types/firestore.d.ts index 46388c383e52..710990718372 100644 --- a/handwritten/firestore/types/firestore.d.ts +++ b/handwritten/firestore/types/firestore.d.ts @@ -12457,11 +12457,15 @@ declare namespace FirebaseFirestore { createFrom(query: Query): Pipeline; /** * Set the pipeline's source to the in-memory documents specified by the given records. + * + * @param documents An array of objects/records specifying the in-memory documents. + */ + literals(documents: Array>): Pipeline; + /** + * Set the pipeline's source to the in-memory documents specified by the given records. + * + * @param options Options defining how this LiteralsSource stage is evaluated. */ - literals( - documents: Array>, - options?: LiteralsStageOptions, - ): Pipeline; literals(options: LiteralsStageOptions): Pipeline; } @@ -12886,6 +12890,13 @@ declare namespace FirebaseFirestore { * @return A new {@link Pipeline} object with this stage appended to the stage list. */ delete(): Pipeline; + /** + * Performs a delete operation on documents from previous stages. + * + * @param options - Options defining how this Delete stage is evaluated. + * @return A new {@link Pipeline} object with this stage appended to the stage list. + */ + delete(options: DeleteStageOptions): Pipeline; /** * Performs an update operation using documents from previous stages. * @@ -12913,12 +12924,10 @@ declare namespace FirebaseFirestore { /** * Performs an update operation using documents from previous stages. * - * @param fieldsMap - Map of field transformations to apply. + * @param options - Options defining how this Update stage is evaluated. * @return A new {@link Pipeline} object with this stage appended to the stage list. */ - update( - fieldsMap: Record | Map, - ): Pipeline; + update(options: UpdateStageOptions): Pipeline; /** * Performs an insert operation on documents from previous stages. * @@ -12932,13 +12941,6 @@ declare namespace FirebaseFirestore { * @return A new {@link Pipeline} object with this stage appended to the stage list. */ upsert(): Pipeline; - /** - * Performs an upsert operation on documents from previous stages. - * - * @param options - Options defining how this Upsert stage is evaluated. - * @return A new {@link Pipeline} object with this stage appended to the stage list. - */ - upsert(options: UpsertStageOptions): Pipeline; /** * Performs an upsert operation on documents from previous stages. * @@ -12953,25 +12955,23 @@ declare namespace FirebaseFirestore { /** * Performs an upsert operation on documents from previous stages. * - * @param additionalFields - Additional fields to apply on upsert. * @param options - Options defining how this Upsert stage is evaluated. * @return A new {@link Pipeline} object with this stage appended to the stage list. */ - upsert( - additionalFields: AliasedExpression[], - options?: UpsertStageOptions, - ): Pipeline; + upsert(options: UpsertStageOptions): Pipeline; /** * Appends a literals stage to the pipeline. * * @param documents An array of objects/records specifying in-memory documents. + * @return A new {@link Pipeline} object with this stage appended to the stage list. + */ + literals(documents: Array>): Pipeline; + /** + * Appends a literals stage to the pipeline. + * * @param options Options defining how this LiteralsSource stage is evaluated. - * @return A new {@code Pipeline} object with this stage appended to the stage list. + * @return A new {@link Pipeline} object with this stage appended to the stage list. */ - literals( - documents: Array>, - options?: LiteralsStageOptions, - ): Pipeline; literals(options: LiteralsStageOptions): Pipeline; /** * Filters the documents from previous stages to only include those matching the specified {@link @@ -14154,6 +14154,21 @@ declare namespace FirebaseFirestore { // separator?: string; // }; + /** + * Options defining how a Delete stage is evaluated. + */ + export type DeleteStageOptions = StageOptions; + + /** + * Options defining how an Update stage is evaluated. + */ + export type UpdateStageOptions = StageOptions & { + /** + * The field transformations to apply to each document being updated. + */ + transformedFields?: AliasedExpression[]; + }; + /** * Options defining how an Insert stage is evaluated. */ @@ -14179,7 +14194,10 @@ declare namespace FirebaseFirestore { * Options defining how a LiteralsSource stage is evaluated. */ export type LiteralsStageOptions = StageOptions & { - documents?: Array>; + /** + * An array of objects specifying the in-memory documents that will be the source of this pipeline. + */ + documents: Array>; }; /** From c73b2cb99a4bea3663a49eef1e0d1194cfc94489 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Wed, 7 Oct 2026 20:44:04 +0000 Subject: [PATCH 2/8] refactor(firestore): split insert into insert() and insert(options) overloads Follow the precedent set by database() for stages with no required params, consistent with delete(). --- handwritten/firestore/dev/src/pipelines/pipelines.ts | 7 +++++++ handwritten/firestore/types/firestore.d.ts | 8 +++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/handwritten/firestore/dev/src/pipelines/pipelines.ts b/handwritten/firestore/dev/src/pipelines/pipelines.ts index 75dcb470f783..be38c3c8f650 100644 --- a/handwritten/firestore/dev/src/pipelines/pipelines.ts +++ b/handwritten/firestore/dev/src/pipelines/pipelines.ts @@ -1863,12 +1863,19 @@ export class Pipeline implements firestore.Pipelines.Pipeline { return this._addStage(new UpdateStage(internalOptions)); } + /** + * Performs an insert operation on documents from previous stages. + * + * @return A new {@code Pipeline} object with this stage appended to the stage list. + */ + insert(): Pipeline; /** * Performs an insert operation on documents from previous stages. * * @param options - Options defining how this Insert stage is evaluated. * @return A new {@code Pipeline} object with this stage appended to the stage list. */ + insert(options: firestore.Pipelines.InsertStageOptions): Pipeline; insert(options?: firestore.Pipelines.InsertStageOptions): Pipeline { return this._addStage( new InsertStage((options ?? {}) as InternalInsertStageOptions), diff --git a/handwritten/firestore/types/firestore.d.ts b/handwritten/firestore/types/firestore.d.ts index 710990718372..f89f52be5c91 100644 --- a/handwritten/firestore/types/firestore.d.ts +++ b/handwritten/firestore/types/firestore.d.ts @@ -12928,13 +12928,19 @@ declare namespace FirebaseFirestore { * @return A new {@link Pipeline} object with this stage appended to the stage list. */ update(options: UpdateStageOptions): Pipeline; + /** + * Performs an insert operation on documents from previous stages. + * + * @return A new {@link Pipeline} object with this stage appended to the stage list. + */ + insert(): Pipeline; /** * Performs an insert operation on documents from previous stages. * * @param options - Options defining how this Insert stage is evaluated. * @return A new {@link Pipeline} object with this stage appended to the stage list. */ - insert(options?: InsertStageOptions): Pipeline; + insert(options: InsertStageOptions): Pipeline; /** * Performs an upsert operation on documents from previous stages. * From 4667b41797b78707ff4900a038f40635928c1f44 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Wed, 7 Oct 2026 20:55:17 +0000 Subject: [PATCH 3/8] refactor(firestore): remove deprecated transforms option from UpsertStageOptions transforms was an earlier name for additionalFields that was never released in the Node or Web SDKs, so the deprecated alias is unnecessary API surface. --- .../firestore/dev/src/pipelines/stage.ts | 10 +++--- .../firestore/dev/test/pipelines/pipeline.ts | 35 ------------------- handwritten/firestore/types/firestore.d.ts | 4 --- 3 files changed, 4 insertions(+), 45 deletions(-) diff --git a/handwritten/firestore/dev/src/pipelines/stage.ts b/handwritten/firestore/dev/src/pipelines/stage.ts index 3c3cb369c502..ea8c4d5bd017 100644 --- a/handwritten/firestore/dev/src/pipelines/stage.ts +++ b/handwritten/firestore/dev/src/pipelines/stage.ts @@ -1055,11 +1055,10 @@ export class InsertStage implements Stage { */ export type InternalUpsertStageOptions = Omit< firestore.Pipelines.UpsertStageOptions, - 'additionalFields' | 'transforms' | 'collection' | 'documentIdExpression' + 'additionalFields' | 'collection' | 'documentIdExpression' > & { additionalFields?: AliasedExpression[] | firestore.Pipelines.AliasedExpression[]; - transforms?: AliasedExpression[] | firestore.Pipelines.AliasedExpression[]; collection?: string | CollectionReference | firestore.CollectionReference; documentIdExpression?: string | Expression | firestore.Pipelines.Expression; }; @@ -1077,10 +1076,9 @@ export class UpsertStage implements Stage { constructor(private readonly options: InternalUpsertStageOptions = {}) { const opts = options; - const resolvedFields = (opts.additionalFields ?? - opts.transforms ?? - []) as AliasedExpression[]; - this.additionalFields = selectablesToMap(resolvedFields); + this.additionalFields = selectablesToMap( + (opts.additionalFields ?? []) as AliasedExpression[], + ); if (opts.collection) { this.collectionPath = typeof opts.collection === 'string' diff --git a/handwritten/firestore/dev/test/pipelines/pipeline.ts b/handwritten/firestore/dev/test/pipelines/pipeline.ts index 3967083f0d7a..d367f4060127 100644 --- a/handwritten/firestore/dev/test/pipelines/pipeline.ts +++ b/handwritten/firestore/dev/test/pipelines/pipeline.ts @@ -771,40 +771,5 @@ describe('execute(Pipeline|PipelineExecuteOptions)', () => { collection: {referenceValue: '/users'}, }); }); - - it('supports deprecated transforms option for backward compatibility', async () => { - const spy = sinon.fake.returns(stream()); - const firestore = await createInstance({ - executePipeline: spy, - }); - - await firestore - .pipeline() - .collection('foo') - .upsert({ - collection: 'users', - transforms: [constant(true).as('active')], - }) - .execute(); - - const stages = - spy.args[FIRST_CALL][EXECUTE_PIPELINE_REQUEST]['structuredPipeline'][ - 'pipeline' - ]['stages']; - - expect(stages[1].name).to.equal('upsert'); - expect(stages[1].args).to.deep.equal([ - { - mapValue: { - fields: { - active: {booleanValue: true}, - }, - }, - }, - ]); - expect(stages[1].options).to.deep.equal({ - collection: {referenceValue: '/users'}, - }); - }); }); }); diff --git a/handwritten/firestore/types/firestore.d.ts b/handwritten/firestore/types/firestore.d.ts index f89f52be5c91..0c3d57b25b96 100644 --- a/handwritten/firestore/types/firestore.d.ts +++ b/handwritten/firestore/types/firestore.d.ts @@ -14188,10 +14188,6 @@ declare namespace FirebaseFirestore { */ export type UpsertStageOptions = StageOptions & { additionalFields?: AliasedExpression[]; - /** - * @deprecated Use `additionalFields` instead. - */ - transforms?: AliasedExpression[]; collection?: string | CollectionReference; documentIdExpression?: string | Expression; }; From 02b0247ac23ee3acfef678fa826f722448246de8 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Wed, 7 Oct 2026 21:00:00 +0000 Subject: [PATCH 4/8] refactor(firestore): remove literals from Pipeline, keep it on PipelineSource only literals is a source stage (LiteralsSource). It was only ever used and tested as the first stage via PipelineSource, and the Web SDK exposes it only on PipelineSource. --- .../firestore/dev/src/pipelines/pipelines.ts | 47 +++---------------- .../firestore/dev/test/pipelines/stage.ts | 12 ----- handwritten/firestore/types/firestore.d.ts | 14 ------ 3 files changed, 6 insertions(+), 67 deletions(-) diff --git a/handwritten/firestore/dev/src/pipelines/pipelines.ts b/handwritten/firestore/dev/src/pipelines/pipelines.ts index be38c3c8f650..6cd1b8821cc3 100644 --- a/handwritten/firestore/dev/src/pipelines/pipelines.ts +++ b/handwritten/firestore/dev/src/pipelines/pipelines.ts @@ -126,21 +126,6 @@ import { ReflectionObject, } from 'protobufjs'; -/** - * Normalizes the arguments of the `literals(documents)` and - * `literals(options)` overloads into a single options object. - * @private - * @internal - */ -function toLiteralsStageOptions( - documentsOrOptions: - Array> | firestore.Pipelines.LiteralsStageOptions, -): InternalLiteralsStageOptions { - return Array.isArray(documentsOrOptions) - ? {documents: documentsOrOptions} - : documentsOrOptions; -} - /** * Represents the source of a Firestore `Pipeline`. */ @@ -163,9 +148,12 @@ export class PipelineSource implements firestore.Pipelines.PipelineSource { documentsOrOptions: Array> | firestore.Pipelines.LiteralsStageOptions, ): Pipeline { - return new Pipeline(this.db, [ - new LiteralsSource(toLiteralsStageOptions(documentsOrOptions)), - ]); + const options: InternalLiteralsStageOptions = Array.isArray( + documentsOrOptions, + ) + ? {documents: documentsOrOptions} + : documentsOrOptions; + return new Pipeline(this.db, [new LiteralsSource(options)]); } /** @@ -1921,29 +1909,6 @@ export class Pipeline implements firestore.Pipelines.Pipeline { return this._addStage(new UpsertStage(additionalFieldOrOptions ?? {})); } - /** - * Appends a literals stage to the pipeline. - * - * @param documents An array of objects/records specifying in-memory documents. - * @return A new {@code Pipeline} object with this stage appended to the stage list. - */ - literals(documents: Array>): Pipeline; - /** - * Appends a literals stage to the pipeline. - * - * @param options Options defining how this LiteralsSource stage is evaluated. - * @return A new {@code Pipeline} object with this stage appended to the stage list. - */ - literals(options: firestore.Pipelines.LiteralsStageOptions): Pipeline; - literals( - documentsOrOptions: - Array> | firestore.Pipelines.LiteralsStageOptions, - ): Pipeline { - return this._addStage( - new LiteralsSource(toLiteralsStageOptions(documentsOrOptions)), - ); - } - /** * Adds a raw stage to the pipeline. * diff --git a/handwritten/firestore/dev/test/pipelines/stage.ts b/handwritten/firestore/dev/test/pipelines/stage.ts index b138f89e8991..3ced33a7ef2f 100644 --- a/handwritten/firestore/dev/test/pipelines/stage.ts +++ b/handwritten/firestore/dev/test/pipelines/stage.ts @@ -257,18 +257,6 @@ describe('stage option serialization', () => { rawOptions, }), }, - { - name: 'literals stage', - pipeline: firestore => - firestore - .pipeline() - .database() - .literals({ - documents: [{foo: 'bar'}], - rawOptions, - }), - stageIndex: 1, - }, { name: 'delete stage', pipeline: firestore => diff --git a/handwritten/firestore/types/firestore.d.ts b/handwritten/firestore/types/firestore.d.ts index 0c3d57b25b96..a5acec77cd8b 100644 --- a/handwritten/firestore/types/firestore.d.ts +++ b/handwritten/firestore/types/firestore.d.ts @@ -12965,20 +12965,6 @@ declare namespace FirebaseFirestore { * @return A new {@link Pipeline} object with this stage appended to the stage list. */ upsert(options: UpsertStageOptions): Pipeline; - /** - * Appends a literals stage to the pipeline. - * - * @param documents An array of objects/records specifying in-memory documents. - * @return A new {@link Pipeline} object with this stage appended to the stage list. - */ - literals(documents: Array>): Pipeline; - /** - * Appends a literals stage to the pipeline. - * - * @param options Options defining how this LiteralsSource stage is evaluated. - * @return A new {@link Pipeline} object with this stage appended to the stage list. - */ - literals(options: LiteralsStageOptions): Pipeline; /** * Filters the documents from previous stages to only include those matching the specified {@link * BooleanExpression}. From 08a4a00fcf68b205dfd7cea9dd038dfcc76ae95f Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Wed, 7 Oct 2026 21:28:03 +0000 Subject: [PATCH 5/8] fix(firestore): throw on FieldValue sentinels in literals stage documents FieldValue sentinels (serverTimestamp(), increment(), arrayUnion(), arrayRemove(), delete(), ...) cannot be represented in an encoded pipeline. Previously they were silently dropped from literal documents (and a document containing only sentinels failed with an INTERNAL serialization error). Literal documents are now validated recursively and sentinels throw the same error as the Web SDK: Function literals() called with invalid data. () can only be used with update() and set() (found in field ) Nested values with user data are now also validated, not just top-level fields. --- .../firestore/dev/src/pipelines/stage.ts | 31 +++++++-- .../firestore/dev/test/pipelines/stage.ts | 66 ++++++++++++++++++- 2 files changed, 89 insertions(+), 8 deletions(-) diff --git a/handwritten/firestore/dev/src/pipelines/stage.ts b/handwritten/firestore/dev/src/pipelines/stage.ts index ea8c4d5bd017..d7218ca7be09 100644 --- a/handwritten/firestore/dev/src/pipelines/stage.ts +++ b/handwritten/firestore/dev/src/pipelines/stage.ts @@ -18,6 +18,8 @@ import api = protos.google.firestore.v1; import {DocumentReference} from '../reference/document-reference'; import {hasUserData, ProtoSerializable, Serializer} from '../serializer'; +import {FieldTransform} from '../field-value'; +import {isPlainObject} from '../util'; import { AggregateFunction, @@ -1159,12 +1161,27 @@ export class LiteralsSource implements Stage { } _validateUserData(ignoreUndefinedProperties: boolean): void { - this.options.documents.forEach(doc => { - Object.values(doc).forEach(val => { - if (hasUserData(val)) { - val._validateUserData(ignoreUndefinedProperties); - } - }); - }); + const validate = (value: unknown, path: string[]): void => { + if (value instanceof FieldTransform) { + // FieldValue sentinels (serverTimestamp(), increment(), delete(), ...) + // have no representation in a pipeline, so they cannot be encoded. + // The error matches the Web SDK. + const location = + path.length > 0 ? ` (found in field ${path.join('.')})` : ''; + throw new Error( + `Function literals() called with invalid data. ${value.methodName}() ` + + `can only be used with update() and set()${location}`, + ); + } else if (hasUserData(value)) { + value._validateUserData(ignoreUndefinedProperties); + } else if (Array.isArray(value)) { + value.forEach(element => validate(element, path)); + } else if (isPlainObject(value)) { + Object.entries(value as Record).forEach( + ([key, fieldValue]) => validate(fieldValue, [...path, key]), + ); + } + }; + this.options.documents.forEach(doc => validate(doc, [])); } } diff --git a/handwritten/firestore/dev/test/pipelines/stage.ts b/handwritten/firestore/dev/test/pipelines/stage.ts index 3ced33a7ef2f..3417e0e54ba2 100644 --- a/handwritten/firestore/dev/test/pipelines/stage.ts +++ b/handwritten/firestore/dev/test/pipelines/stage.ts @@ -19,7 +19,7 @@ import {expect} from 'chai'; import * as sinon from 'sinon'; import {createInstance, stream} from '../util/helpers'; import {google} from '../../protos/firestore_v1_proto_api'; -import {Pipelines, Firestore} from '../../src'; +import {Pipelines, Firestore, FieldValue} from '../../src'; import Pipeline = Pipelines.Pipeline; import field = Pipelines.field; import sum = Pipelines.sum; @@ -444,4 +444,68 @@ describe('stage _validateUserData', () => { }) .execute(); }); + + describe('literals stage rejects FieldValue sentinels', () => { + const cases: Array<{ + name: string; + doc: Record; + expected: string; + }> = [ + { + name: 'top-level serverTimestamp()', + doc: {a: 1, ts: FieldValue.serverTimestamp()}, + expected: + 'Function literals() called with invalid data. FieldValue.serverTimestamp() can only be used with update() and set() (found in field ts)', + }, + { + name: 'increment()', + doc: {n: FieldValue.increment(1)}, + expected: + 'Function literals() called with invalid data. FieldValue.increment() can only be used with update() and set() (found in field n)', + }, + { + name: 'arrayUnion()', + doc: {arr: FieldValue.arrayUnion('x')}, + expected: + 'Function literals() called with invalid data. FieldValue.arrayUnion() can only be used with update() and set() (found in field arr)', + }, + { + name: 'delete()', + doc: {gone: FieldValue.delete()}, + expected: + 'Function literals() called with invalid data. FieldValue.delete() can only be used with update() and set() (found in field gone)', + }, + { + name: 'nested in a map', + doc: {a: {b: FieldValue.serverTimestamp()}}, + expected: + 'Function literals() called with invalid data. FieldValue.serverTimestamp() can only be used with update() and set() (found in field a.b)', + }, + { + name: 'inside an array', + doc: {arr: [1, FieldValue.serverTimestamp()]}, + expected: + 'Function literals() called with invalid data. FieldValue.serverTimestamp() can only be used with update() and set() (found in field arr)', + }, + ]; + + cases.forEach(({name, doc, expected}) => { + it(name, async () => { + const firestore = await createInstance(); + expect(() => { + void firestore.pipeline().literals([doc]).execute(); + }).to.throw(expected); + }); + }); + + it('accepts literals without sentinels', async () => { + const spy = sinon.fake.returns(stream()); + const firestore = await createInstance({executePipeline: spy}); + await firestore + .pipeline() + .literals([{a: 1, nested: {b: [1, 2]}, c: constant(3)}]) + .execute(); + expect(spy.callCount).to.equal(1); + }); + }); }); From 3f6f12dabe55d07a82292d377b3ddbee60f26538 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Wed, 7 Oct 2026 21:45:27 +0000 Subject: [PATCH 6/8] fix(firestore): allow null values in literals stage validation hasUserData() dereferences its argument, so a null value in a literal document crashed _validateUserData with a TypeError. --- handwritten/firestore/dev/src/pipelines/stage.ts | 2 ++ handwritten/firestore/dev/test/pipelines/stage.ts | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/handwritten/firestore/dev/src/pipelines/stage.ts b/handwritten/firestore/dev/src/pipelines/stage.ts index d7218ca7be09..b95fc47d27bc 100644 --- a/handwritten/firestore/dev/src/pipelines/stage.ts +++ b/handwritten/firestore/dev/src/pipelines/stage.ts @@ -1172,6 +1172,8 @@ export class LiteralsSource implements Stage { `Function literals() called with invalid data. ${value.methodName}() ` + `can only be used with update() and set()${location}`, ); + } else if (value === null) { + // null is a valid literal value. } else if (hasUserData(value)) { value._validateUserData(ignoreUndefinedProperties); } else if (Array.isArray(value)) { diff --git a/handwritten/firestore/dev/test/pipelines/stage.ts b/handwritten/firestore/dev/test/pipelines/stage.ts index 3417e0e54ba2..aed3614556d9 100644 --- a/handwritten/firestore/dev/test/pipelines/stage.ts +++ b/handwritten/firestore/dev/test/pipelines/stage.ts @@ -503,7 +503,9 @@ describe('stage _validateUserData', () => { const firestore = await createInstance({executePipeline: spy}); await firestore .pipeline() - .literals([{a: 1, nested: {b: [1, 2]}, c: constant(3)}]) + .literals([ + {a: 1, nested: {b: [1, 2, null], n: null}, c: constant(3), d: null}, + ]) .execute(); expect(spy.callCount).to.equal(1); }); From 031d9c74787d59b2ae8b68c18a65d3f40b812bd2 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Wed, 7 Oct 2026 21:45:32 +0000 Subject: [PATCH 7/8] fix(firestore): auto-wrap nested objects and arrays in literals documents The backend only evaluates expressions in the top-level fields of a literal document, so nested expressions (e.g. {a: {b: add(...)}}) failed with "Value type is not supported: FUNCTION_VALUE". Encode each document as a top-level mapValue whose fields are converted with valueToDefaultExpr, so nested objects and arrays become map() and array() expressions, matching rawStage() and expression arguments. Undefined values are now rejected unless ignoreUndefinedProperties is enabled, preserving the previous serializer behavior. --- .../firestore/dev/src/pipelines/stage.ts | 38 ++++++++-- .../firestore/dev/system-test/pipeline.ts | 38 ++++++++++ .../firestore/dev/test/pipelines/pipeline.ts | 70 +++++++++++++++++-- .../firestore/dev/test/pipelines/stage.ts | 36 ++++++++++ 4 files changed, 172 insertions(+), 10 deletions(-) diff --git a/handwritten/firestore/dev/src/pipelines/stage.ts b/handwritten/firestore/dev/src/pipelines/stage.ts index b95fc47d27bc..ebc0eaf741f6 100644 --- a/handwritten/firestore/dev/src/pipelines/stage.ts +++ b/handwritten/firestore/dev/src/pipelines/stage.ts @@ -28,11 +28,16 @@ import { Expression, Field, field, + MapValue, Ordering, } from './expression'; import {OptionsUtil} from './options-util'; import {CollectionReference} from '../reference/collection-reference'; -import {validateUserDataHelper, selectablesToMap} from './pipeline-util'; +import { + validateUserDataHelper, + selectablesToMap, + valueToDefaultExpr, +} from './pipeline-util'; import {Pipeline} from './pipelines'; /** @@ -1146,9 +1151,20 @@ export class LiteralsSource implements Stage { constructor(private options: InternalLiteralsStageOptions) {} _toProto(serializer: Serializer): api.Pipeline.IStage { - const args: api.IValue[] = this.options.documents.map(doc => - serializer.encodeValue(doc)!, - ); + // Each document is sent as a top-level mapValue. The backend only + // evaluates expressions in the top-level fields of a literal document, so + // nested objects and arrays are converted to map() and array() expressions + // (the same conversion used for objects and arrays passed to expressions + // and to rawStage()). This allows expressions at any depth. + const args: api.IValue[] = this.options.documents.map(doc => { + const fields = new Map(); + for (const [key, value] of Object.entries(doc)) { + if (value !== undefined) { + fields.set(key, valueToDefaultExpr(value)); + } + } + return new MapValue(fields)._toProto(serializer); + }); return { name: this.name, args, @@ -1162,16 +1178,26 @@ export class LiteralsSource implements Stage { _validateUserData(ignoreUndefinedProperties: boolean): void { const validate = (value: unknown, path: string[]): void => { + const location = + path.length > 0 ? ` (found in field ${path.join('.')})` : ''; if (value instanceof FieldTransform) { // FieldValue sentinels (serverTimestamp(), increment(), delete(), ...) // have no representation in a pipeline, so they cannot be encoded. // The error matches the Web SDK. - const location = - path.length > 0 ? ` (found in field ${path.join('.')})` : ''; throw new Error( `Function literals() called with invalid data. ${value.methodName}() ` + `can only be used with update() and set()${location}`, ); + } else if (value === undefined) { + // Undefined values are omitted from the encoded document, which is + // only allowed when `ignoreUndefinedProperties` is enabled. + if (!ignoreUndefinedProperties) { + throw new Error( + 'Function literals() called with invalid data. Cannot use ' + + `"undefined" as a Firestore value${location}. If you want to ` + + 'ignore undefined values, enable `ignoreUndefinedProperties`.', + ); + } } else if (value === null) { // null is a valid literal value. } else if (hasUserData(value)) { diff --git a/handwritten/firestore/dev/system-test/pipeline.ts b/handwritten/firestore/dev/system-test/pipeline.ts index dcc3324dd21b..6120c48bc2a0 100644 --- a/handwritten/firestore/dev/system-test/pipeline.ts +++ b/handwritten/firestore/dev/system-test/pipeline.ts @@ -828,6 +828,44 @@ describe.skipClassic('Pipeline class', () => { expect(res.results.length).to.equal(1); expect(res.results[0].data()).to.deep.equal({base: 10, computed: 35}); }); + + it('can execute literals stage with nested expressions', async () => { + const res = await firestore + .pipeline() + .literals([ + { + nested: {sum: add(constant(1), constant(2)), label: 'x'}, + arr: [1, add(constant(3), constant(4))], + deep: {a: {b: [{c: add(constant(5), constant(6))}]}}, + }, + ]) + .execute(); + expect(res.results.length).to.equal(1); + expect(res.results[0].data()).to.deep.equal({ + nested: {sum: 3, label: 'x'}, + arr: [1, 7], + deep: {a: {b: [{c: 11}]}}, + }); + }); + + it('can execute literals stage with nested constant values', async () => { + const res = await firestore + .pipeline() + .literals([ + { + emptyMap: {}, + emptyArr: [], + nested: {a: 1, b: [true, null, 'str'], c: {d: 2.5}}, + }, + ]) + .execute(); + expect(res.results.length).to.equal(1); + expect(res.results[0].data()).to.deep.equal({ + emptyMap: {}, + emptyArr: [], + nested: {a: 1, b: [true, null, 'str'], c: {d: 2.5}}, + }); + }); }); it('empty snapshot as expected', async () => { diff --git a/handwritten/firestore/dev/test/pipelines/pipeline.ts b/handwritten/firestore/dev/test/pipelines/pipeline.ts index d367f4060127..2864cc19385c 100644 --- a/handwritten/firestore/dev/test/pipelines/pipeline.ts +++ b/handwritten/firestore/dev/test/pipelines/pipeline.ts @@ -410,7 +410,9 @@ describe('execute(Pipeline|PipelineExecuteOptions)', () => { doubled: Pipelines.multiply(constant(10), constant(2)), nested: { val: constant('hello'), + sum: Pipelines.add(constant(1), constant(2)), }, + arr: [1, Pipelines.add(constant(3), constant(4))], }, ]) .execute(); @@ -420,6 +422,9 @@ describe('execute(Pipeline|PipelineExecuteOptions)', () => { 'pipeline' ]['stages']; + // The document itself is a mapValue, but nested objects and arrays are + // wrapped in map() and array() expressions, because the backend only + // evaluates expressions in the top-level fields of a literal document. expect(stages[0].name).to.equal('literals'); expect(stages[0].args).to.deep.equal([ { @@ -433,10 +438,67 @@ describe('execute(Pipeline|PipelineExecuteOptions)', () => { }, }, nested: { - mapValue: { - fields: { - val: {stringValue: 'hello'}, - }, + functionValue: { + name: 'map', + args: [ + {stringValue: 'val'}, + {stringValue: 'hello'}, + {stringValue: 'sum'}, + { + functionValue: { + name: 'add', + args: [{integerValue: 1}, {integerValue: 2}], + }, + }, + ], + }, + }, + arr: { + functionValue: { + name: 'array', + args: [ + {integerValue: 1}, + { + functionValue: { + name: 'add', + args: [{integerValue: 3}, {integerValue: 4}], + }, + }, + ], + }, + }, + }, + }, + }, + ]); + }); + + it('omits undefined literal fields when ignoreUndefinedProperties is set', async () => { + const spy = sinon.fake.returns(stream()); + const firestore = await createInstance( + {executePipeline: spy}, + {ignoreUndefinedProperties: true}, + ); + + await firestore + .pipeline() + .literals([{a: 1, b: undefined, nested: {c: undefined, d: 2}}]) + .execute(); + + const stages = + spy.args[FIRST_CALL][EXECUTE_PIPELINE_REQUEST]['structuredPipeline'][ + 'pipeline' + ]['stages']; + + expect(stages[0].args).to.deep.equal([ + { + mapValue: { + fields: { + a: {integerValue: 1}, + nested: { + functionValue: { + name: 'map', + args: [{stringValue: 'd'}, {integerValue: 2}], }, }, }, diff --git a/handwritten/firestore/dev/test/pipelines/stage.ts b/handwritten/firestore/dev/test/pipelines/stage.ts index aed3614556d9..6cb7ccb7e5f8 100644 --- a/handwritten/firestore/dev/test/pipelines/stage.ts +++ b/handwritten/firestore/dev/test/pipelines/stage.ts @@ -510,4 +510,40 @@ describe('stage _validateUserData', () => { expect(spy.callCount).to.equal(1); }); }); + + describe('literals stage rejects undefined by default', () => { + const cases: Array<{ + name: string; + doc: Record; + expected: string; + }> = [ + { + name: 'top-level undefined', + doc: {a: 1, b: undefined}, + expected: + 'Function literals() called with invalid data. Cannot use "undefined" as a Firestore value (found in field b).', + }, + { + name: 'nested undefined', + doc: {a: {b: undefined}}, + expected: + 'Function literals() called with invalid data. Cannot use "undefined" as a Firestore value (found in field a.b).', + }, + { + name: 'undefined inside an array', + doc: {arr: [1, undefined]}, + expected: + 'Function literals() called with invalid data. Cannot use "undefined" as a Firestore value (found in field arr).', + }, + ]; + + cases.forEach(({name, doc, expected}) => { + it(name, async () => { + const firestore = await createInstance(); + expect(() => { + void firestore.pipeline().literals([doc]).execute(); + }).to.throw(expected); + }); + }); + }); }); From 2e84058e2975f438e575d785cbba3c2e8db3cad8 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Wed, 7 Oct 2026 23:21:09 +0000 Subject: [PATCH 8/8] docs(firestore): document insert/upsert target documents and the atomic option Describe how collection and documentIdExpression determine the target document, that a string documentIdExpression is a field name (use constant() for a fixed ID), that upsert requires collection when documentIdExpression is set, and what atomic does. Behavior verified against nightly; adds system tests for the field-name semantics and the upsert collection requirement. --- .../firestore/dev/system-test/pipeline.ts | 39 +++++++++++ handwritten/firestore/types/firestore.d.ts | 68 ++++++++++++++++++- 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/handwritten/firestore/dev/system-test/pipeline.ts b/handwritten/firestore/dev/system-test/pipeline.ts index 6120c48bc2a0..13c79b91e9af 100644 --- a/handwritten/firestore/dev/system-test/pipeline.ts +++ b/handwritten/firestore/dev/system-test/pipeline.ts @@ -799,6 +799,45 @@ describe.skipClassic('Pipeline class', () => { expect(snap.get('title')).to.equal('Tx Upsert Book'); }); + it('treats a string documentIdExpression as a field name', async () => { + const targetCol = getTestRoot(); + const res = await firestore + .pipeline() + .collection(dmlCol.path) + .where(equal(field('__name__').documentId(), 'book1')) + .addFields(constant('id_from_field').as('customId')) + .insert({ + collection: targetCol.path, + documentIdExpression: 'customId', + }) + .execute({atomic: true}); + expectResults(res, {documents_modified: 1}); + + const docSnap = await targetCol.doc('id_from_field').get(); + expect(docSnap.exists).to.be.true; + expect(docSnap.get('title')).to.equal( + "The Hitchhiker's Guide to the Galaxy", + ); + // A literal document ID 'customId' is not used. + expect((await targetCol.doc('customId').get()).exists).to.be.false; + }); + + it('upsert with documentIdExpression requires a collection', async () => { + let error: Error | undefined; + try { + await firestore + .pipeline() + .collection(dmlCol.path) + .upsert({documentIdExpression: constant('some_id')}) + .execute({atomic: true}); + } catch (e) { + error = e as Error; + } + expect(error?.message).to.contain( + "The 'upsert(...)' stage must specify a collection when a document id is provided.", + ); + }); + it('can execute pipeline with literals stage', async () => { const res = await firestore .pipeline() diff --git a/handwritten/firestore/types/firestore.d.ts b/handwritten/firestore/types/firestore.d.ts index a5acec77cd8b..394f4b2bd686 100644 --- a/handwritten/firestore/types/firestore.d.ts +++ b/handwritten/firestore/types/firestore.d.ts @@ -13666,7 +13666,17 @@ declare namespace FirebaseFirestore { */ indexMode?: 'recommended'; /** - * Indicates that the pipeline will be executed atomically. + * If `true`, the pipeline is executed in a new read-write transaction that + * is committed automatically when the pipeline completes, so all writes + * made by DML stages (`insert`, `upsert`, `update`, `delete`) succeed or + * fail together. + * + * If `false` or omitted, DML writes are applied per document without a + * transaction, and the pipeline may partially succeed. Some DML pipelines + * (e.g. an `upsert` from a `literals()` source) can only be executed + * atomically, and are otherwise rejected with "The non-transactional DML + * operation was rejected because it was impossible to guarantee + * document-level atomicity." */ atomic?: boolean; /** @@ -14163,18 +14173,74 @@ declare namespace FirebaseFirestore { /** * Options defining how an Insert stage is evaluated. + * + * Each input document is written as a new document; the insert fails if the + * target document already exists. The target is determined by `collection` + * and `documentIdExpression`: + * + * - Only `collection` set: the input document's ID is reused under + * `collection`. If the input document has no ID (e.g. it came from + * `literals()`), an ID is generated automatically. + * - Only `documentIdExpression` set: the evaluated ID is used under the input + * document's parent collection. + * - Both set: the evaluated ID is used under `collection`. + * + * If neither is set, each document is written back to its input document's + * path, which fails for documents read from the database because they + * already exist. */ export type InsertStageOptions = StageOptions & { + /** + * The collection to insert documents into. If omitted, each document is + * inserted into the parent collection of its input document. + */ collection?: string | CollectionReference; + /** + * An expression that is evaluated against each input document to produce + * the ID of the document to insert. It must evaluate to a document ID + * (e.g. `'abc123'`), not a document path or `DocumentReference`. + * + * A string is treated as a field name: `'id'` is equivalent to + * `field('id')`, and uses the value of each input document's `id` field as + * its document ID. To use a fixed document ID, pass `constant('abc123')`. + */ documentIdExpression?: string | Expression; }; /** * Options defining how an Upsert stage is evaluated. + * + * Each input document is written to its target document. A target document + * that doesn't exist is created; an existing one is replaced entirely + * (stored fields are not merged). The target is determined by `collection` + * and `documentIdExpression`: + * + * - Neither set: each document is written back to its input document's path. + * - Only `collection` set: the input document's ID is reused under + * `collection`. + * - Both set: the evaluated ID is used under `collection`. */ export type UpsertStageOptions = StageOptions & { + /** + * Fields to add to each input document before it is written. A field with + * the same name as an existing input field overwrites that field. + */ additionalFields?: AliasedExpression[]; + /** + * The collection to upsert documents into. If omitted, each document is + * written to the parent collection of its input document. Required when + * `documentIdExpression` is set. + */ collection?: string | CollectionReference; + /** + * An expression that is evaluated against each input document to produce + * the ID of the target document. It must evaluate to a document ID + * (e.g. `'abc123'`), not a document path or `DocumentReference`. Requires + * `collection`. + * + * A string is treated as a field name: `'id'` is equivalent to + * `field('id')`. To use a fixed document ID, pass `constant('abc123')`. + */ documentIdExpression?: string | Expression; };