From 876df2cf03706977e860b90af8a388f15fcbae85 Mon Sep 17 00:00:00 2001 From: Emily M Klassen Date: Tue, 18 Feb 2025 19:18:53 -0800 Subject: [PATCH 1/6] docs(NODE-6765): update comments for findOneAndUpdate --- src/collection.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/collection.ts b/src/collection.ts index 19a0682d8b2..7817132f164 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -966,8 +966,12 @@ export class Collection { /** * Find a document and update it in one atomic operation. Requires a write lock for the duration of the operation. * + * The value of `update` can be either: + * - UpdateFilter - A document that contains update operator expressions, + * - Document[] - an aggregation pipeline. + * * @param filter - The filter used to select the document to update - * @param update - Update operations to be performed on the document + * @param update - The modifications to apply * @param options - Optional settings for the command */ async findOneAndUpdate( From c1e4b17d18377084ecb83423a30e2e93dacc3aa0 Mon Sep 17 00:00:00 2001 From: Emily M Klassen Date: Tue, 18 Feb 2025 19:19:51 -0800 Subject: [PATCH 2/6] feat(NODE-6765): explicitly allow an aggregation pipeline in findOneAndUpdate --- src/collection.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/collection.ts b/src/collection.ts index 7817132f164..7207022f3c5 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -976,26 +976,26 @@ export class Collection { */ async findOneAndUpdate( filter: Filter, - update: UpdateFilter, + update: UpdateFilter | Document[], options: FindOneAndUpdateOptions & { includeResultMetadata: true } ): Promise>; async findOneAndUpdate( filter: Filter, - update: UpdateFilter, + update: UpdateFilter | Document[], options: FindOneAndUpdateOptions & { includeResultMetadata: false } ): Promise | null>; async findOneAndUpdate( filter: Filter, - update: UpdateFilter, + update: UpdateFilter | Document[], options: FindOneAndUpdateOptions ): Promise | null>; async findOneAndUpdate( filter: Filter, - update: UpdateFilter + update: UpdateFilter | Document[] ): Promise | null>; async findOneAndUpdate( filter: Filter, - update: UpdateFilter, + update: UpdateFilter | Document[], options?: FindOneAndUpdateOptions ): Promise | ModifyResult | null> { return await executeOperation( From ac151f35b7e9317ae26f7bb594bb64fd2485b197 Mon Sep 17 00:00:00 2001 From: Emily M Klassen Date: Tue, 18 Feb 2025 19:20:37 -0800 Subject: [PATCH 3/6] test(NODE-6765): findOneAndUpdate with aggregation pipeline integration --- test/integration/crud/find_and_modify.test.ts | 184 ++++++++++++++++++ 1 file changed, 184 insertions(+) diff --git a/test/integration/crud/find_and_modify.test.ts b/test/integration/crud/find_and_modify.test.ts index 16764f020ea..a6ea91de433 100644 --- a/test/integration/crud/find_and_modify.test.ts +++ b/test/integration/crud/find_and_modify.test.ts @@ -324,6 +324,190 @@ describe('Collection (#findOneAnd...)', function () { }); }); }); + + context('when updating with an aggregation pipeline', function () { + context('when passing includeResultMetadata: true', function () { + let client; + let collection; + + beforeEach(async function () { + client = this.configuration.newClient({}, { maxPoolSize: 1 }); + collection = client.db('test').collection('findAndModifyTest'); + await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); + }); + + afterEach(async function () { + await collection.drop(); + await client?.close(); + }); + + it('returns the raw result', async function () { + const result = await collection.findOneAndUpdate( + { a: 1 }, + [{ $set: { a: { $add: [0, '$a'] } } }], + { includeResultMetadata: true } + ); + expect(result.value.b).to.equal(1); + }); + }); + + context('when no options are passed', function () { + let client; + let collection; + + beforeEach(async function () { + client = this.configuration.newClient({}, { maxPoolSize: 1 }); + collection = client.db('test').collection('findAndModifyTest'); + await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); + }); + + afterEach(async function () { + await collection.drop(); + await client?.close(); + }); + + context('when there is a match', function () { + it('returns the modified document', async function () { + const result = await collection.findOneAndUpdate({ a: 1 }, [ + { $set: { a: { $add: [0, '$a'] } } } + ]); + expect(result.b).to.equal(1); + }); + }); + + context('when there is no match', function () { + it('returns null', async function () { + const result = await collection.findOneAndUpdate({ a: 2 }, [ + { $set: { a: { $add: [0, '$a'] } } } + ]); + expect(result).to.equal(null); + }); + }); + }); + + context('when passing an object id filter', function () { + let client; + let collection; + const started: CommandStartedEvent[] = []; + + beforeEach(async function () { + client = this.configuration.newClient({}, { maxPoolSize: 1, monitorCommands: true }); + client.on('commandStarted', function (event) { + if (event.commandName === 'findAndModify') started.push(event); + }); + collection = client.db('test').collection('findAndModifyTest'); + await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); + }); + + afterEach(async function () { + await collection.drop(); + await client?.close(); + }); + + it('does not support object ids as a query predicate', async function () { + const oid = new ObjectId(); + const error = await collection + .findOneAndUpdate(oid, [{ $set: { a: { $add: [0, '$a'] } } }]) + .catch(error => error); + expect(error).to.be.instanceOf(MongoServerError); + expect(started).to.have.lengthOf(1); + expect(started[0].command).to.have.property('query', oid); + }); + }); + + context('when passing in a non-primary read preference', { + metadata: { + requires: { topology: ['replicaset'] } + }, + test: function () { + let client; + let collection; + + beforeEach(async function () { + client = this.configuration.newClient( + { readPreference: 'secondary' }, + { maxPoolSize: 1 } + ); + collection = client.db('test').collection('findAndModifyTest'); + await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); + }); + + afterEach(async function () { + await collection.drop(); + await client?.close(); + }); + + it('returns the raw result', async function () { + const result = await collection.findOneAndUpdate( + { a: 1 }, + [{ $set: { a: { $add: [0, '$a'] } } }], + { includeResultMetadata: true } + ); + expect(result.value.b).to.equal(1); + }); + } + }); + + context('when passing in writeConcern', function () { + let client; + let collection; + const started: CommandStartedEvent[] = []; + + beforeEach(async function () { + client = this.configuration.newClient({}, { maxPoolSize: 1, monitorCommands: true }); + client.on('commandStarted', function (event) { + if (event.commandName === 'findAndModify') started.push(event); + }); + }); + + afterEach(async function () { + await collection.drop(); + await client?.close(); + }); + + context('when provided at the operation level', function () { + beforeEach(async function () { + collection = client.db('test').collection('findAndModifyTest'); + await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); + }); + + it('passes through the writeConcern', async function () { + await collection.findOneAndUpdate({}, [{ $set: { a: { $add: [0, '$a'] } } }], { + writeConcern: { j: 1 } + }); + expect(started[0].command.writeConcern).to.deep.equal({ j: 1 }); + }); + }); + + context('when provided at the collection level', function () { + beforeEach(async function () { + collection = client + .db('test') + .collection('findAndModifyTest', { writeConcern: { j: 1 } }); + await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); + }); + + it('passes through the writeConcern', async function () { + await collection.findOneAndUpdate({}, [{ $set: { a: { $add: [0, '$a'] } } }]); + expect(started[0].command.writeConcern).to.deep.equal({ j: 1 }); + }); + }); + + context('when provided at the db level', function () { + beforeEach(async function () { + collection = client + .db('test', { writeConcern: { j: 1 } }) + .collection('findAndModifyTest'); + await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); + }); + + it('passes through the writeConcern', async function () { + await collection.findOneAndUpdate({}, [{ $set: { a: { $add: [0, '$a'] } } }]); + expect(started[0].command.writeConcern).to.deep.equal({ j: 1 }); + }); + }); + }); + }); }); describe('#findOneAndReplace', function () { From 945e6d90909a6f9574ce63a4a315ee9e6da535ab Mon Sep 17 00:00:00 2001 From: Emily M Klassen Date: Tue, 18 Feb 2025 19:21:14 -0800 Subject: [PATCH 4/6] test(NODE-6765): add type test for aggregation in findOneAndUpdate --- test/types/community/collection/findX.test-d.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/types/community/collection/findX.test-d.ts b/test/types/community/collection/findX.test-d.ts index 8720c949400..0de99ee7d49 100644 --- a/test/types/community/collection/findX.test-d.ts +++ b/test/types/community/collection/findX.test-d.ts @@ -388,3 +388,14 @@ expectType | null>( } ) ); + +// the update operator can be an aggregation pipeline +expectType | null>( + await coll.findOneAndUpdate({ a: 3 }, [ + { + $set: { + a: 5 + } + } + ]) +); From b94edf131ce20cd32c3e1998ee1f2eb040ccfa82 Mon Sep 17 00:00:00 2001 From: bailey Date: Wed, 19 Feb 2025 09:58:35 -0700 Subject: [PATCH 5/6] remove unnecessary tests and make documentation more specific --- src/collection.ts | 6 +- test/integration/crud/find_and_modify.test.ts | 172 +++--------------- 2 files changed, 27 insertions(+), 151 deletions(-) diff --git a/src/collection.ts b/src/collection.ts index 7207022f3c5..a2df98ae2da 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -968,7 +968,11 @@ export class Collection { * * The value of `update` can be either: * - UpdateFilter - A document that contains update operator expressions, - * - Document[] - an aggregation pipeline. + * - Document[] - an aggregation pipeline consisting of the following stages: + * - $addFields and its alias $set + * - $project and its alias $unset + * - $replaceRoot and its alias $replaceWith. + * See the [findAndModify command documentation](https://www.mongodb.com/docs/manual/reference/command/findAndModify) for details. * * @param filter - The filter used to select the document to update * @param update - The modifications to apply diff --git a/test/integration/crud/find_and_modify.test.ts b/test/integration/crud/find_and_modify.test.ts index a6ea91de433..7684cbbe58e 100644 --- a/test/integration/crud/find_and_modify.test.ts +++ b/test/integration/crud/find_and_modify.test.ts @@ -1,6 +1,6 @@ import { expect } from 'chai'; -import { type CommandStartedEvent, MongoServerError, ObjectId } from '../../mongodb'; +import { Collection, type CommandStartedEvent, MongoClient, MongoServerError, ObjectId } from '../../mongodb'; import { setupDatabase } from '../shared'; describe('Collection (#findOneAnd...)', function () { @@ -327,8 +327,8 @@ describe('Collection (#findOneAnd...)', function () { context('when updating with an aggregation pipeline', function () { context('when passing includeResultMetadata: true', function () { - let client; - let collection; + let client: MongoClient; + let collection: Collection<{ a: number, b: number }>; beforeEach(async function () { client = this.configuration.newClient({}, { maxPoolSize: 1 }); @@ -341,19 +341,22 @@ describe('Collection (#findOneAnd...)', function () { await client?.close(); }); - it('returns the raw result', async function () { - const result = await collection.findOneAndUpdate( + it('the aggregation pipeline updates the matching document', async function () { + const { value: { + _id, + ...document + } } = await collection.findOneAndUpdate( { a: 1 }, - [{ $set: { a: { $add: [0, '$a'] } } }], - { includeResultMetadata: true } + [{ $set: { a: { $add: [1, '$a'] } } }], + { includeResultMetadata: true, returnDocument: 'after' } ); - expect(result.value.b).to.equal(1); + expect(document).to.deep.equal({ a: 2, b: 1 }); }); }); - context('when no options are passed', function () { - let client; - let collection; + context('when passing includeResultMetadata: false', function () { + let client: MongoClient; + let collection: Collection<{ a: number, b: number }>; beforeEach(async function () { client = this.configuration.newClient({}, { maxPoolSize: 1 }); @@ -366,145 +369,14 @@ describe('Collection (#findOneAnd...)', function () { await client?.close(); }); - context('when there is a match', function () { - it('returns the modified document', async function () { - const result = await collection.findOneAndUpdate({ a: 1 }, [ - { $set: { a: { $add: [0, '$a'] } } } - ]); - expect(result.b).to.equal(1); - }); - }); - - context('when there is no match', function () { - it('returns null', async function () { - const result = await collection.findOneAndUpdate({ a: 2 }, [ - { $set: { a: { $add: [0, '$a'] } } } - ]); - expect(result).to.equal(null); - }); - }); - }); - - context('when passing an object id filter', function () { - let client; - let collection; - const started: CommandStartedEvent[] = []; - - beforeEach(async function () { - client = this.configuration.newClient({}, { maxPoolSize: 1, monitorCommands: true }); - client.on('commandStarted', function (event) { - if (event.commandName === 'findAndModify') started.push(event); - }); - collection = client.db('test').collection('findAndModifyTest'); - await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); - }); - - afterEach(async function () { - await collection.drop(); - await client?.close(); - }); - - it('does not support object ids as a query predicate', async function () { - const oid = new ObjectId(); - const error = await collection - .findOneAndUpdate(oid, [{ $set: { a: { $add: [0, '$a'] } } }]) - .catch(error => error); - expect(error).to.be.instanceOf(MongoServerError); - expect(started).to.have.lengthOf(1); - expect(started[0].command).to.have.property('query', oid); - }); - }); - - context('when passing in a non-primary read preference', { - metadata: { - requires: { topology: ['replicaset'] } - }, - test: function () { - let client; - let collection; - - beforeEach(async function () { - client = this.configuration.newClient( - { readPreference: 'secondary' }, - { maxPoolSize: 1 } - ); - collection = client.db('test').collection('findAndModifyTest'); - await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); - }); - - afterEach(async function () { - await collection.drop(); - await client?.close(); - }); - - it('returns the raw result', async function () { - const result = await collection.findOneAndUpdate( - { a: 1 }, - [{ $set: { a: { $add: [0, '$a'] } } }], - { includeResultMetadata: true } - ); - expect(result.value.b).to.equal(1); - }); - } - }); - - context('when passing in writeConcern', function () { - let client; - let collection; - const started: CommandStartedEvent[] = []; - - beforeEach(async function () { - client = this.configuration.newClient({}, { maxPoolSize: 1, monitorCommands: true }); - client.on('commandStarted', function (event) { - if (event.commandName === 'findAndModify') started.push(event); - }); - }); - - afterEach(async function () { - await collection.drop(); - await client?.close(); - }); - - context('when provided at the operation level', function () { - beforeEach(async function () { - collection = client.db('test').collection('findAndModifyTest'); - await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); - }); - - it('passes through the writeConcern', async function () { - await collection.findOneAndUpdate({}, [{ $set: { a: { $add: [0, '$a'] } } }], { - writeConcern: { j: 1 } - }); - expect(started[0].command.writeConcern).to.deep.equal({ j: 1 }); - }); - }); - - context('when provided at the collection level', function () { - beforeEach(async function () { - collection = client - .db('test') - .collection('findAndModifyTest', { writeConcern: { j: 1 } }); - await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); - }); - - it('passes through the writeConcern', async function () { - await collection.findOneAndUpdate({}, [{ $set: { a: { $add: [0, '$a'] } } }]); - expect(started[0].command.writeConcern).to.deep.equal({ j: 1 }); - }); - }); - - context('when provided at the db level', function () { - beforeEach(async function () { - collection = client - .db('test', { writeConcern: { j: 1 } }) - .collection('findAndModifyTest'); - await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); - }); - - it('passes through the writeConcern', async function () { - await collection.findOneAndUpdate({}, [{ $set: { a: { $add: [0, '$a'] } } }]); - expect(started[0].command.writeConcern).to.deep.equal({ j: 1 }); - }); + it('the aggregation pipeline updates the matching document', async function () { + const { + _id, + ...document + } = await collection.findOneAndUpdate({ a: 1 }, [ + { $set: { a: { $add: [1, '$a'] } } } + ], { returnDocument: 'after' }); + expect(document).to.deep.equal({ a: 2, b: 1 }); }); }); }); From 002c392e810c835b2e3aeb7cf32dc1a5c74b888f Mon Sep 17 00:00:00 2001 From: bailey Date: Wed, 19 Feb 2025 10:34:40 -0700 Subject: [PATCH 6/6] fix lint and CI --- test/integration/crud/find_and_modify.test.ts | 69 ++++++++++++------- 1 file changed, 46 insertions(+), 23 deletions(-) diff --git a/test/integration/crud/find_and_modify.test.ts b/test/integration/crud/find_and_modify.test.ts index 7684cbbe58e..18ab93cf3da 100644 --- a/test/integration/crud/find_and_modify.test.ts +++ b/test/integration/crud/find_and_modify.test.ts @@ -1,6 +1,12 @@ import { expect } from 'chai'; -import { Collection, type CommandStartedEvent, MongoClient, MongoServerError, ObjectId } from '../../mongodb'; +import { + type Collection, + type CommandStartedEvent, + type MongoClient, + MongoServerError, + ObjectId +} from '../../mongodb'; import { setupDatabase } from '../shared'; describe('Collection (#findOneAnd...)', function () { @@ -328,7 +334,7 @@ describe('Collection (#findOneAnd...)', function () { context('when updating with an aggregation pipeline', function () { context('when passing includeResultMetadata: true', function () { let client: MongoClient; - let collection: Collection<{ a: number, b: number }>; + let collection: Collection<{ a: number; b: number }>; beforeEach(async function () { client = this.configuration.newClient({}, { maxPoolSize: 1 }); @@ -341,22 +347,32 @@ describe('Collection (#findOneAnd...)', function () { await client?.close(); }); - it('the aggregation pipeline updates the matching document', async function () { - const { value: { - _id, - ...document - } } = await collection.findOneAndUpdate( - { a: 1 }, - [{ $set: { a: { $add: [1, '$a'] } } }], - { includeResultMetadata: true, returnDocument: 'after' } - ); - expect(document).to.deep.equal({ a: 2, b: 1 }); - }); + it( + 'the aggregation pipeline updates the matching document', + { + requires: { + mongodb: '>4.0' + } + }, + async function () { + const { + value: { _id, ...document } + } = await collection.findOneAndUpdate( + { a: 1 }, + [{ $set: { a: { $add: [1, '$a'] } } }], + { + includeResultMetadata: true, + returnDocument: 'after' + } + ); + expect(document).to.deep.equal({ a: 2, b: 1 }); + } + ); }); context('when passing includeResultMetadata: false', function () { let client: MongoClient; - let collection: Collection<{ a: number, b: number }>; + let collection: Collection<{ a: number; b: number }>; beforeEach(async function () { client = this.configuration.newClient({}, { maxPoolSize: 1 }); @@ -369,15 +385,22 @@ describe('Collection (#findOneAnd...)', function () { await client?.close(); }); - it('the aggregation pipeline updates the matching document', async function () { - const { - _id, - ...document - } = await collection.findOneAndUpdate({ a: 1 }, [ - { $set: { a: { $add: [1, '$a'] } } } - ], { returnDocument: 'after' }); - expect(document).to.deep.equal({ a: 2, b: 1 }); - }); + it( + 'the aggregation pipeline updates the matching document', + { + requires: { + mongodb: '>4.0' + } + }, + async function () { + const { _id, ...document } = await collection.findOneAndUpdate( + { a: 1 }, + [{ $set: { a: { $add: [1, '$a'] } } }], + { returnDocument: 'after' } + ); + expect(document).to.deep.equal({ a: 2, b: 1 }); + } + ); }); }); });