diff --git a/src/collection.ts b/src/collection.ts index 1981cfb38d5..e74e5bcb6e0 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -100,12 +100,7 @@ import { } from './utils'; import { WriteConcern, type WriteConcernOptions } from './write_concern'; -/** - * @public - * @deprecated This type will be completely removed and findOneAndUpdate, - * findOneAndDelete, and findOneAndReplace will then return the - * actual result document. - */ +/** @public */ export interface ModifyResult { value: WithId | null; lastErrorObject?: Document; @@ -825,10 +820,23 @@ export class Collection { * @param filter - The filter used to select the document to remove * @param options - Optional settings for the command */ + async findOneAndDelete( + filter: Filter, + options: FindOneAndDeleteOptions & { includeResultMetadata: true } + ): Promise>; + async findOneAndDelete( + filter: Filter, + options: FindOneAndDeleteOptions & { includeResultMetadata: false } + ): Promise | null>; + async findOneAndDelete( + filter: Filter, + options: FindOneAndDeleteOptions + ): Promise>; + async findOneAndDelete(filter: Filter): Promise>; async findOneAndDelete( filter: Filter, options?: FindOneAndDeleteOptions - ): Promise> { + ): Promise | ModifyResult | null> { return executeOperation( this.client, new FindOneAndDeleteOperation( @@ -846,11 +854,30 @@ export class Collection { * @param replacement - The Document that replaces the matching document * @param options - Optional settings for the command */ + async findOneAndReplace( + filter: Filter, + replacement: WithoutId, + options: FindOneAndReplaceOptions & { includeResultMetadata: true } + ): Promise>; + async findOneAndReplace( + filter: Filter, + replacement: WithoutId, + options: FindOneAndReplaceOptions & { includeResultMetadata: false } + ): Promise | null>; + async findOneAndReplace( + filter: Filter, + replacement: WithoutId, + options: FindOneAndReplaceOptions + ): Promise>; + async findOneAndReplace( + filter: Filter, + replacement: WithoutId + ): Promise>; async findOneAndReplace( filter: Filter, replacement: WithoutId, options?: FindOneAndReplaceOptions - ): Promise> { + ): Promise | ModifyResult | null> { return executeOperation( this.client, new FindOneAndReplaceOperation( @@ -869,11 +896,30 @@ export class Collection { * @param update - Update operations to be performed on the document * @param options - Optional settings for the command */ + async findOneAndUpdate( + filter: Filter, + update: UpdateFilter, + options: FindOneAndUpdateOptions & { includeResultMetadata: true } + ): Promise>; + async findOneAndUpdate( + filter: Filter, + update: UpdateFilter, + options: FindOneAndUpdateOptions & { includeResultMetadata: false } + ): Promise | null>; + async findOneAndUpdate( + filter: Filter, + update: UpdateFilter, + options: FindOneAndUpdateOptions + ): Promise>; + async findOneAndUpdate( + filter: Filter, + update: UpdateFilter + ): Promise>; async findOneAndUpdate( filter: Filter, update: UpdateFilter, options?: FindOneAndUpdateOptions - ): Promise> { + ): Promise | ModifyResult | null> { return executeOperation( this.client, new FindOneAndUpdateOperation( diff --git a/src/operations/find_and_modify.ts b/src/operations/find_and_modify.ts index 0e9a59e85af..59000221142 100644 --- a/src/operations/find_and_modify.ts +++ b/src/operations/find_and_modify.ts @@ -29,6 +29,8 @@ export interface FindOneAndDeleteOptions extends CommandOperationOptions { sort?: Sort; /** Map of parameter names and values that can be accessed using $$var (requires MongoDB 5.0). */ let?: Document; + /** Return the raw result document instead of the ModifyResult */ + includeResultMetadata?: boolean; } /** @public */ @@ -47,6 +49,8 @@ export interface FindOneAndReplaceOptions extends CommandOperationOptions { upsert?: boolean; /** Map of parameter names and values that can be accessed using $$var (requires MongoDB 5.0). */ let?: Document; + /** Return the raw result document instead of the ModifyResult */ + includeResultMetadata?: boolean; } /** @public */ @@ -67,6 +71,8 @@ export interface FindOneAndUpdateOptions extends CommandOperationOptions { upsert?: boolean; /** Map of parameter names and values that can be accessed using $$var (requires MongoDB 5.0). */ let?: Document; + /** Return the raw result document instead of the ModifyResult */ + includeResultMetadata?: boolean; } /** @internal */ @@ -127,6 +133,8 @@ class FindAndModifyOperation extends CommandOperation { upsert: false }; + options.includeResultMetadata ??= true; + const sort = formatSort(options.sort); if (sort) { this.cmdBase.sort = sort; @@ -205,7 +213,7 @@ class FindAndModifyOperation extends CommandOperation { // Execute the command super.executeCommand(server, session, cmd, (err, result) => { if (err) return callback(err); - return callback(undefined, result); + return callback(undefined, options.includeResultMetadata ? result : result.value ?? null); }); } } diff --git a/test/integration/crud/crud_api.test.ts b/test/integration/crud/crud_api.test.ts index 8e38a2c32e6..b5f97540cec 100644 --- a/test/integration/crud/crud_api.test.ts +++ b/test/integration/crud/crud_api.test.ts @@ -618,112 +618,160 @@ describe('CRUD API', function () { } }); - it('should correctly execute findAndModify methods using crud api', { - // Add a tag that our runner can trigger on - // in this case we are setting that node needs to be higher than 0.10.X to run - metadata: { - requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] } - }, + describe('#findOneAndDelete', function () { + let collection; - test: function (done) { - client.connect(function (err, client) { - const db = client.db(); + beforeEach(async function () { + await client.connect(); + collection = client.db().collection('findAndModifyTest'); + }); - // - // findOneAndRemove method - // ------------------------------------------------- - const findOneAndRemove = function () { - db.collection('t5_1').insertMany( - [{ a: 1, b: 1 }], - { writeConcern: { w: 1 } }, - function (err, r) { - expect(err).to.not.exist; - expect(r).property('insertedCount').to.equal(1); + afterEach(async function () { + await collection.drop(); + }); - db.collection('t5_1').findOneAndDelete( - { a: 1 }, - { projection: { b: 1 }, sort: { a: 1 } }, - function (err, r) { - expect(err).to.not.exist; - test.equal(1, r.lastErrorObject.n); - test.equal(1, r.value.b); + context('when includeResultMetadata is true', function () { + beforeEach(async function () { + await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); + }); - findOneAndReplace(); - } - ); - } - ); - }; + it('returns the modify result', async function () { + const result = await collection.findOneAndDelete( + { a: 1 }, + { projection: { b: 1 }, sort: { a: 1 } } + ); + expect(result?.lastErrorObject.n).to.equal(1); + expect(result?.value.b).to.equal(1); + }); + }); - // - // findOneAndRemove method - // ------------------------------------------------- - const findOneAndReplace = function () { - db.collection('t5_2').insertMany( - [{ a: 1, b: 1 }], - { writeConcern: { w: 1 } }, - function (err, r) { - expect(err).to.not.exist; - expect(r).property('insertedCount').to.equal(1); + context('when includeResultMetadata is false', function () { + beforeEach(async function () { + await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); + }); - db.collection('t5_2').findOneAndReplace( - { a: 1 }, - { c: 1, b: 1 }, - { - projection: { b: 1, c: 1 }, - sort: { a: 1 }, - returnDocument: ReturnDocument.AFTER, - upsert: true - }, - function (err, r) { - expect(err).to.not.exist; - test.equal(1, r.lastErrorObject.n); - test.equal(1, r.value.b); - test.equal(1, r.value.c); + it('returns the deleted document', async function () { + const result = await collection.findOneAndDelete( + { a: 1 }, + { projection: { b: 1 }, sort: { a: 1 }, includeResultMetadata: false } + ); + expect(result?.b).to.equal(1); + }); + }); + }); - findOneAndUpdate(); - } - ); - } - ); - }; + describe('#findOneAndReplace', function () { + let collection; - // - // findOneAndRemove method - // ------------------------------------------------- - const findOneAndUpdate = function () { - db.collection('t5_3').insertMany( - [{ a: 1, b: 1 }], - { writeConcern: { w: 1 } }, - function (err, r) { - expect(err).to.not.exist; - expect(r).property('insertedCount').to.equal(1); + beforeEach(async function () { + await client.connect(); + collection = client.db().collection('findAndModifyTest'); + }); - db.collection('t5_3').findOneAndUpdate( - { a: 1 }, - { $set: { d: 1 } }, - { - projection: { b: 1, d: 1 }, - sort: { a: 1 }, - returnDocument: ReturnDocument.AFTER, - upsert: true - }, - function (err, r) { - expect(err).to.not.exist; - test.equal(1, r.lastErrorObject.n); - test.equal(1, r.value.b); - test.equal(1, r.value.d); + afterEach(async function () { + await collection.drop(); + }); - client.close(done); - } - ); - } - ); - }; + context('when includeResultMetadata is true', function () { + beforeEach(async function () { + await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); + }); + + it('returns the modify result', async function () { + const result = await collection.findOneAndReplace( + { a: 1 }, + { c: 1, b: 1 }, + { + projection: { b: 1, c: 1 }, + sort: { a: 1 }, + returnDocument: ReturnDocument.AFTER, + upsert: true + } + ); + expect(result?.lastErrorObject.n).to.equal(1); + expect(result?.value.b).to.equal(1); + expect(result?.value.c).to.equal(1); + }); + }); - findOneAndRemove(); + context('when includeResultMetadata is false', function () { + beforeEach(async function () { + await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); }); - } + + it('returns the replaced document', async function () { + const result = await collection.findOneAndReplace( + { a: 1 }, + { c: 1, b: 1 }, + { + projection: { b: 1, c: 1 }, + sort: { a: 1 }, + returnDocument: ReturnDocument.AFTER, + upsert: true, + includeResultMetadata: false + } + ); + expect(result?.b).to.equal(1); + expect(result?.c).to.equal(1); + }); + }); + }); + + describe('#findOneAndUpdate', function () { + let collection; + + beforeEach(async function () { + await client.connect(); + collection = client.db().collection('findAndModifyTest'); + }); + + afterEach(async function () { + await collection.drop(); + }); + + context('when includeResultMetadata is true', function () { + beforeEach(async function () { + await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); + }); + + it('returns the modify result', async function () { + const result = await collection.findOneAndUpdate( + { a: 1 }, + { $set: { d: 1 } }, + { + projection: { b: 1, d: 1 }, + sort: { a: 1 }, + returnDocument: ReturnDocument.AFTER, + upsert: true + } + ); + expect(result?.lastErrorObject.n).to.equal(1); + expect(result?.value.b).to.equal(1); + expect(result?.value.d).to.equal(1); + }); + }); + + context('when includeResultMetadata is false', function () { + beforeEach(async function () { + await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); + }); + + it('returns the replaced document', async function () { + const result = await collection.findOneAndUpdate( + { a: 1 }, + { $set: { d: 1 } }, + { + projection: { b: 1, d: 1 }, + sort: { a: 1 }, + returnDocument: ReturnDocument.AFTER, + upsert: true, + includeResultMetadata: false + } + ); + expect(result?.b).to.equal(1); + expect(result?.d).to.equal(1); + }); + }); }); it('should correctly execute removeMany with no selector', { diff --git a/test/integration/crud/find.test.js b/test/integration/crud/find.test.js index e3281eb499b..6803ece2cbf 100644 --- a/test/integration/crud/find.test.js +++ b/test/integration/crud/find.test.js @@ -781,126 +781,6 @@ describe('Find', function () { } }); - /** - * Test findOneAndUpdate a document - */ - it('shouldCorrectlyfindOneAndUpdateDocument', { - metadata: { - requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] } - }, - - test: function (done) { - var configuration = this.configuration; - var db = client.db(configuration.db); - db.dropCollection('test_find_and_modify_a_document_1') - .catch(() => null) - .finally(() => { - db.createCollection('test_find_and_modify_a_document_1', function (err, collection) { - expect(err).to.not.exist; - - // Test return new document on change - collection.insert({ a: 1, b: 2 }, configuration.writeConcernMax(), function (err) { - expect(err).to.not.exist; - - // Let's modify the document in place - collection.findOneAndUpdate( - { a: 1 }, - { $set: { b: 3 } }, - { returnDocument: ReturnDocument.AFTER }, - function (err, updated_doc) { - expect(err).to.not.exist; - test.equal(1, updated_doc.value.a); - test.equal(3, updated_doc.value.b); - - // Test return old document on change - collection.insert( - { a: 2, b: 2 }, - configuration.writeConcernMax(), - function (err) { - expect(err).to.not.exist; - - // Let's modify the document in place - collection.findOneAndUpdate( - { a: 2 }, - { $set: { b: 3 } }, - configuration.writeConcernMax(), - function (err, result) { - expect(err).to.not.exist; - test.equal(2, result.value.a); - test.equal(2, result.value.b); - - // Test remove object on change - collection.insert( - { a: 3, b: 2 }, - configuration.writeConcernMax(), - function (err) { - expect(err).to.not.exist; - // Let's modify the document in place - collection.findOneAndUpdate( - { a: 3 }, - { $set: { b: 3 } }, - { remove: true }, - function (err, updated_doc) { - expect(err).to.not.exist; - test.equal(3, updated_doc.value.a); - test.equal(2, updated_doc.value.b); - - // Let's upsert! - collection.findOneAndUpdate( - { a: 4 }, - { $set: { b: 3 } }, - { returnDocument: ReturnDocument.AFTER, upsert: true }, - function (err, updated_doc) { - expect(err).to.not.exist; - test.equal(4, updated_doc.value.a); - test.equal(3, updated_doc.value.b); - - // Test selecting a subset of fields - collection.insert( - { a: 100, b: 101 }, - configuration.writeConcernMax(), - function (err, r) { - expect(err).to.not.exist; - - collection.findOneAndUpdate( - { a: 100 }, - { $set: { b: 5 } }, - { - returnDocument: ReturnDocument.AFTER, - projection: { b: 1 } - }, - function (err, updated_doc) { - expect(err).to.not.exist; - test.equal(2, Object.keys(updated_doc.value).length); - test.equal( - r.insertedIds[0].toHexString(), - updated_doc.value._id.toHexString() - ); - test.equal(5, updated_doc.value.b); - test.equal('undefined', typeof updated_doc.value.a); - client.close(done); - } - ); - } - ); - } - ); - } - ); - } - ); - } - ); - } - ); - } - ); - }); - }); - }); - } - }); - /** * Test findOneAndUpdate a document with fields */ @@ -925,8 +805,8 @@ describe('Find', function () { { $set: { b: 3 } }, { returnDocument: ReturnDocument.AFTER, projection: { a: 1 } }, function (err, updated_doc) { - test.equal(2, Object.keys(updated_doc.value).length); - test.equal(1, updated_doc.value.a); + expect(Object.keys(updated_doc.value).length).to.equal(2); + expect(updated_doc.value.a).to.equal(1); client.close(done); } ); @@ -981,53 +861,6 @@ describe('Find', function () { } }); - /** - * Test findOneAndUpdate a document - */ - it('Should Correctly Handle findOneAndUpdate Duplicate Key Error', { - metadata: { - requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] } - }, - - test: function (done) { - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); - client.connect(function (err, client) { - expect(err).to.not.exist; - var db = client.db(configuration.db); - db.createCollection('findOneAndUpdateDuplicateKeyError', function (err, collection) { - expect(err).to.not.exist; - collection.createIndex( - ['name', 1], - { unique: true, writeConcern: { w: 1 } }, - function (err) { - expect(err).to.not.exist; - // Test return new document on change - collection.insert( - [{ name: 'test1' }, { name: 'test2' }], - configuration.writeConcernMax(), - function (err) { - expect(err).to.not.exist; - // Let's modify the document in place - collection.findOneAndUpdate( - { name: 'test1' }, - { $set: { name: 'test2' } }, - {}, - function (err, updated_doc) { - expect(err).to.exist; - expect(updated_doc).to.not.exist; - client.close(done); - } - ); - } - ); - } - ); - }); - }); - } - }); - it('Should correctly return null when attempting to modify a non-existing document', { metadata: { requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] } @@ -1168,8 +1001,8 @@ describe('Find', function () { { $set: { b: 3 } }, { returnDocument: ReturnDocument.AFTER }, function (err, result) { - test.equal(2, result.value.a); - test.equal(3, result.value.b); + expect(result.value.a).to.equal(2); + expect(result.value.b).to.equal(3); p_client.close(done); } ); @@ -1260,12 +1093,12 @@ describe('Find', function () { { $set: { 'c.c': 100 } }, { returnDocument: ReturnDocument.AFTER }, function (err, item) { - test.equal(doc._id.toString(), item.value._id.toString()); - test.equal(doc.a, item.value.a); - test.equal(doc.b, item.value.b); - test.equal(doc.c.a, item.value.c.a); - test.equal(doc.c.b, item.value.c.b); - test.equal(100, item.value.c.c); + expect(item.value._id.toString()).to.equal(doc._id.toString()); + expect(item.value.a).to.equal(doc.a); + expect(item.value.b).to.equal(doc.b); + expect(item.value.c.a).to.equal(doc.c.a); + expect(item.value.c.b).to.equal(doc.c.b); + expect(item.value.c.c).to.equal(100); client.close(done); } ); @@ -2327,8 +2160,8 @@ describe('Find', function () { { $set: { b: 3 } }, { returnDocument: ReturnDocument.AFTER }, function (err, updated_doc) { - test.equal(1, updated_doc.value.a); - test.equal(3, updated_doc.value.b); + expect(updated_doc.value.a).to.equal(1); + expect(updated_doc.value.b).to.equal(3); client.close(done); } diff --git a/test/integration/crud/find_and_modify.test.js b/test/integration/crud/find_and_modify.test.js deleted file mode 100644 index 8bf6a2cb77b..00000000000 --- a/test/integration/crud/find_and_modify.test.js +++ /dev/null @@ -1,376 +0,0 @@ -'use strict'; -const { format: f } = require('util'); - -const { setupDatabase, assert: test } = require(`../shared`); -const { expect } = require('chai'); - -const { ObjectId, MongoServerError } = require('../../mongodb'); - -describe('Find and Modify', function () { - before(function () { - return setupDatabase(this.configuration); - }); - - it('Should correctly execute findOneAndDelete operation and no options passed in', function (done) { - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { - maxPoolSize: 1 - }); - - client.connect().then(function (client) { - const db = client.db(configuration.db); - const col = db.collection('find_one_and_delete_with_promise_no_option'); - col.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }).then(function (r) { - expect(r).property('insertedCount').to.equal(1); - - col - .findOneAndDelete({ a: 1 }) - .then(function (r) { - test.equal(1, r.lastErrorObject.n); - test.equal(1, r.value.b); - - client.close(done); - }) - .catch(function (err) { - test.ok(err != null); - }); - }); - }); - }); - - it('Should correctly execute findOneAndUpate operation and no options passed in', function (done) { - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { - maxPoolSize: 1 - }); - - client.connect().then(function (client) { - const db = client.db(configuration.db); - const col = db.collection('find_one_and_update_with_promise_no_option'); - col.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }).then(function (r) { - expect(r).property('insertedCount').to.equal(1); - - col - .findOneAndUpdate({ a: 1 }, { $set: { a: 1 } }) - .then(function (r) { - test.equal(1, r.lastErrorObject.n); - test.equal(1, r.value.b); - - client.close(done); - }) - .catch(function (err) { - test.ok(err != null); - }); - }); - }); - }); - - it('Should correctly execute findOneAndReplace operation and no options passed in', function (done) { - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { - maxPoolSize: 1 - }); - - client.connect().then(function (client) { - const db = client.db(configuration.db); - const col = db.collection('find_one_and_replace_with_promise_no_option'); - col.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }).then(function (r) { - expect(r).property('insertedCount').to.equal(1); - - col - .findOneAndReplace({ a: 1 }, { a: 1 }) - .then(function (r) { - test.equal(1, r.lastErrorObject.n); - test.equal(1, r.value.b); - - client.close(done); - }) - .catch(function (err) { - test.ok(err != null); - }); - }); - }); - }); - - it('should pass through writeConcern to all findAndModify commands at command level', { - // Add a tag that our runner can trigger on - // in this case we are setting that node needs to be higher than 0.10.X to run - metadata: { - requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] } - }, - - test: function (done) { - var started = []; - var succeeded = []; - - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { - maxPoolSize: 1, - monitorCommands: true - }); - - client.on('commandStarted', function (event) { - if (event.commandName === 'findAndModify') started.push(event); - }); - - client.on('commandSucceeded', function (event) { - if (event.commandName === 'findAndModify') succeeded.push(event); - }); - - client.connect(function (err, client) { - var db = client.db(configuration.db); - expect(err).to.not.exist; - - var collection = db.collection('findAndModifyTEST'); - // Execute findOneAndUpdate - collection.findOneAndUpdate( - {}, - { $set: { a: 1 } }, - { writeConcern: { fsync: 1 } }, - function (err) { - expect(err).to.not.exist; - test.deepEqual({ fsync: 1 }, started[0].command.writeConcern); - - // Cleanup - started = []; - succeeded = []; - - // Execute findOneAndReplace - collection.findOneAndReplace( - {}, - { b: 1 }, - { writeConcern: { fsync: 1 } }, - function (err) { - expect(err).to.not.exist; - test.deepEqual({ fsync: 1 }, started[0].command.writeConcern); - - // Cleanup - started = []; - succeeded = []; - - // Execute findOneAndReplace - collection.findOneAndDelete({}, { writeConcern: { fsync: 1 } }, function (err) { - expect(err).to.not.exist; - test.deepEqual({ fsync: 1 }, started[0].command.writeConcern); - - client.close(done); - }); - } - ); - } - ); - }); - } - }); - - it('should pass through writeConcern to all findAndModify at collection level', { - // Add a tag that our runner can trigger on - // in this case we are setting that node needs to be higher than 0.10.X to run - metadata: { - requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] } - }, - - test: function (done) { - var started = []; - var succeeded = []; - - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { - maxPoolSize: 1, - monitorCommands: true - }); - - client.on('commandStarted', function (event) { - if (event.commandName === 'findAndModify') started.push(event); - }); - - client.on('commandSucceeded', function (event) { - if (event.commandName === 'findAndModify') succeeded.push(event); - }); - - client.connect(function (err, client) { - var db = client.db(configuration.db); - expect(err).to.not.exist; - - var collection = db.collection('findAndModifyTEST', { writeConcern: { fsync: 1 } }); - // Execute findOneAndUpdate - collection.findOneAndUpdate({}, { $set: { a: 1 } }, function (err) { - expect(err).to.not.exist; - test.deepEqual({ fsync: 1 }, started[0].command.writeConcern); - - // Cleanup - started = []; - succeeded = []; - - // Execute findOneAndReplace - collection.findOneAndReplace({}, { b: 1 }, function (err) { - expect(err).to.not.exist; - test.deepEqual({ fsync: 1 }, started[0].command.writeConcern); - - // Cleanup - started = []; - succeeded = []; - - // Execute findOneAndReplace - collection.findOneAndDelete({}, function (err) { - expect(err).to.not.exist; - test.deepEqual({ fsync: 1 }, started[0].command.writeConcern); - - client.close(done); - }); - }); - }); - }); - } - }); - - it('should pass through writeConcern to all findAndModify at db level', { - // Add a tag that our runner can trigger on - // in this case we are setting that node needs to be higher than 0.10.X to run - metadata: { - requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] } - }, - - test: function (done) { - var configuration = this.configuration; - var started = []; - var succeeded = []; - - var url = configuration.url(); - url = url.indexOf('?') !== -1 ? f('%s&%s', url, 'fsync=true') : f('%s?%s', url, 'fsync=true'); - - // Establish connection to db - const client = configuration.newClient(url, { sslValidate: false, monitorCommands: true }); - - client.on('commandStarted', function (event) { - if (event.commandName === 'findAndModify') started.push(event); - }); - - client.on('commandSucceeded', function (event) { - if (event.commandName === 'findAndModify') succeeded.push(event); - }); - - client.connect(function (err, client) { - expect(err).to.not.exist; - var db = client.db(configuration.db); - var collection = db.collection('findAndModifyTEST'); - // Execute findOneAndUpdate - collection.findOneAndUpdate({}, { $set: { a: 1 } }, function (err) { - expect(err).to.not.exist; - test.deepEqual({ fsync: true }, started[0].command.writeConcern); - - // Cleanup - started = []; - succeeded = []; - - // Execute findOneAndReplace - collection.findOneAndReplace({}, { b: 1 }, function (err) { - expect(err).to.not.exist; - test.deepEqual({ fsync: true }, started[0].command.writeConcern); - - // Cleanup - started = []; - succeeded = []; - - // Execute findOneAndReplace - collection.findOneAndDelete({}, function (err) { - expect(err).to.not.exist; - test.deepEqual({ fsync: true }, started[0].command.writeConcern); - - client.close(done); - }); - }); - }); - }); - } - }); - - it('should allow all findAndModify commands with non-primary readPreference', { - // Add a tag that our runner can trigger on - // in this case we are setting that node needs to be higher than 0.10.X to run - metadata: { - requires: { topology: 'replicaset' } - }, - - test: function (done) { - const configuration = this.configuration; - const client = configuration.newClient({ readPreference: 'secondary' }, { maxPoolSize: 1 }); - client.connect((err, client) => { - expect(err).to.not.exist; - const db = client.db(configuration.db); - - const collection = db.collection('findAndModifyTEST'); - // Execute findOneAndUpdate - collection.findOneAndUpdate({}, { $set: { a: 1 } }, err => { - expect(err).to.not.exist; - - client.close(true, done); - }); - }); - } - }); - - it('should not allow atomic operators for findOneAndReplace', async function () { - const client = this.configuration.newClient(); - const db = client.db('fakeDb'); - const collection = db.collection('test'); - const error = await collection - .findOneAndReplace({ a: 1 }, { $set: { a: 14 } }) - .catch(error => error); - expect(error.message).to.match(/must not contain atomic operators/); - await client.close(); - }); - - context('when passed an ObjectId instance as the filter', () => { - let client; - let findAndModifyStarted; - - beforeEach(function () { - client = this.configuration.newClient({ monitorCommands: true }); - findAndModifyStarted = []; - client.on('commandStarted', ev => { - if (ev.commandName === 'findAndModify') findAndModifyStarted.push(ev.command); - }); - }); - - afterEach(async function () { - findAndModifyStarted = undefined; - await client.close(); - }); - - context('findOneAndDelete(oid)', () => { - it('sets the query to be the ObjectId instance', async () => { - const collection = client.db('test').collection('test'); - const oid = new ObjectId(); - const error = await collection.findOneAndDelete(oid).catch(error => error); - expect(error).to.be.instanceOf(MongoServerError); - expect(findAndModifyStarted).to.have.lengthOf(1); - expect(findAndModifyStarted[0]).to.have.property('query', oid); - }); - }); - - context('findOneAndReplace(oid)', () => { - it('sets the query to be the ObjectId instance', async () => { - const collection = client.db('test').collection('test'); - const oid = new ObjectId(); - const error = await collection.findOneAndReplace(oid, {}).catch(error => error); - expect(error).to.be.instanceOf(MongoServerError); - expect(findAndModifyStarted).to.have.lengthOf(1); - expect(findAndModifyStarted[0]).to.have.property('query', oid); - }); - }); - - context('findOneAndUpdate(oid)', () => { - it('sets the query to be the ObjectId instance', async () => { - const collection = client.db('test').collection('test'); - const oid = new ObjectId(); - const error = await collection - .findOneAndUpdate(oid, { $set: { a: 1 } }) - .catch(error => error); - expect(error).to.be.instanceOf(MongoServerError); - expect(findAndModifyStarted).to.have.lengthOf(1); - expect(findAndModifyStarted[0]).to.have.property('query', oid); - }); - }); - }); -}); diff --git a/test/integration/crud/find_and_modify.test.ts b/test/integration/crud/find_and_modify.test.ts new file mode 100644 index 00000000000..2ecb02c178b --- /dev/null +++ b/test/integration/crud/find_and_modify.test.ts @@ -0,0 +1,505 @@ +import { expect } from 'chai'; + +import { type CommandStartedEvent, MongoServerError, ObjectId } from '../../mongodb'; +import { setupDatabase } from '../shared'; + +describe('Collection (#findOneAnd...)', function () { + before(function () { + return setupDatabase(this.configuration); + }); + + describe('#findOneAndDelete', function () { + 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(); + }); + + it('returns the raw result', async function () { + const result = await collection.findOneAndDelete({ a: 1 }); + expect(result.value.b).to.equal(1); + }); + }); + + context('when passing includeResultMetadata: false', 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 deleted document', async function () { + const result = await collection.findOneAndDelete( + { a: 1 }, + { includeResultMetadata: false } + ); + expect(result.b).to.equal(1); + }); + }); + + context('when there is no match', function () { + it('returns null', async function () { + const result = await collection.findOneAndDelete( + { a: 2 }, + { includeResultMetadata: false } + ); + 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.findOneAndDelete(oid).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 writeConcern', function () { + let client; + let collection; + let 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 () { + started = []; + 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.findOneAndDelete({}, { writeConcern: { fsync: 1 } }); + expect(started[0].command.writeConcern).to.deep.equal({ fsync: 1 }); + }); + }); + + context('when provided at the collection level', function () { + beforeEach(async function () { + collection = client + .db('test') + .collection('findAndModifyTest', { writeConcern: { fsync: 1 } }); + await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); + }); + + it('passes through the writeConcern', async function () { + await collection.findOneAndDelete({}); + expect(started[0].command.writeConcern).to.deep.equal({ fsync: 1 }); + }); + }); + + context('when provided at the db level', function () { + beforeEach(async function () { + collection = client + .db('test', { writeConcern: { fsync: 1 } }) + .collection('findAndModifyTest'); + await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); + }); + + it('passes through the writeConcern', async function () { + await collection.findOneAndDelete({}); + expect(started[0].command.writeConcern).to.deep.equal({ fsync: 1 }); + }); + }); + }); + }); + + describe('#findOneAndUpdate', function () { + 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(); + }); + + it('returns the raw result', async function () { + const result = await collection.findOneAndUpdate({ a: 1 }, { $set: { a: 1 } }); + expect(result.value.b).to.equal(1); + }); + }); + + context('when passing includeResultMetadata: false', 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: 1 } }, + { includeResultMetadata: false } + ); + 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: 1 } }, + { includeResultMetadata: false } + ); + 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: 1 } }) + .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: 1 } }); + 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: 1 } }, { writeConcern: { fsync: 1 } }); + expect(started[0].command.writeConcern).to.deep.equal({ fsync: 1 }); + }); + }); + + context('when provided at the collection level', function () { + beforeEach(async function () { + collection = client + .db('test') + .collection('findAndModifyTest', { writeConcern: { fsync: 1 } }); + await collection.insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }); + }); + + it('passes through the writeConcern', async function () { + await collection.findOneAndUpdate({}, { $set: { a: 1 } }); + expect(started[0].command.writeConcern).to.deep.equal({ fsync: 1 }); + }); + }); + + context('when provided at the db level', function () { + beforeEach(async function () { + collection = client + .db('test', { writeConcern: { fsync: 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: 1 } }); + expect(started[0].command.writeConcern).to.deep.equal({ fsync: 1 }); + }); + }); + }); + }); + + describe('#findOneAndReplace', function () { + 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(); + }); + + it('returns the raw result', async function () { + const result = await collection.findOneAndReplace({ a: 1 }, { a: 1 }); + expect(result.value.b).to.equal(1); + }); + }); + + context('when passing includeResultMetadata: false', 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 replaced document', async function () { + const result = await collection.findOneAndReplace( + { a: 1 }, + { a: 1 }, + { includeResultMetadata: false } + ); + expect(result.b).to.equal(1); + }); + }); + + context('when there is no match', function () { + it('returns null', async function () { + const result = await collection.findOneAndReplace( + { a: 2 }, + { a: 1 }, + { includeResultMetadata: false } + ); + 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.findOneAndReplace(oid, {}).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 providing atomic operators', 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 an error', async function () { + const error = await collection + .findOneAndReplace({ a: 1 }, { $set: { a: 14 } }) + .catch(error => error); + expect(error.message).to.match(/must not contain atomic operators/); + }); + }); + + context('when passing in writeConcern', function () { + let client; + let collection; + let 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 () { + started = []; + 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.findOneAndReplace({}, { b: 1 }, { writeConcern: { fsync: 1 } }); + expect(started[0].command.writeConcern).to.deep.equal({ fsync: 1 }); + }); + }); + + context('when provided at the collection level', function () { + beforeEach(async function () { + collection = client + .db('test') + .collection('findAndModifyTest', { writeConcern: { w: 1 } }); + await collection.insertMany([{ a: 1, b: 1 }]); + }); + + it('passes through the writeConcern', async function () { + await collection.findOneAndReplace({}, { b: 1 }); + expect(started[0].command.writeConcern).to.deep.equal({ w: 1 }); + }); + }); + + context('when provided at the db level', function () { + beforeEach(async function () { + collection = client + .db('test', { writeConcern: { w: 1 } }) + .collection('findAndModifyTest'); + await collection.insertMany([{ a: 1, b: 1 }]); + }); + + it('passes through the writeConcern', async function () { + await collection.findOneAndReplace({}, { b: 1 }); + expect(started[0].command.writeConcern).to.deep.equal({ w: 1 }); + }); + }); + }); + }); +}); diff --git a/test/types/community/collection/findX.test-d.ts b/test/types/community/collection/findX.test-d.ts index b1e568a1b2b..8c546cc4e2f 100644 --- a/test/types/community/collection/findX.test-d.ts +++ b/test/types/community/collection/findX.test-d.ts @@ -288,6 +288,7 @@ const fooObjWithArray: FooWithArray = { const fooFilterWithArray: Filter = fooObjWithArray; declare const coll: Collection<{ a: number; b: string }>; +// Passing no options will return the modify result. expectType | null>((await coll.findOneAndDelete({ a: 3 })).value); expectType | null>( (await coll.findOneAndReplace({ a: 3 }, { a: 5, b: 'new string' })).value @@ -305,6 +306,69 @@ expectType | null>( ).value ); +// Passing empty options will return the modify result. +expectType | null>( + (await coll.findOneAndDelete({ a: 3 }, {})).value +); +expectType | null>( + (await coll.findOneAndReplace({ a: 3 }, { a: 5, b: 'new string' }, {})).value +); +expectType | null>( + ( + await coll.findOneAndUpdate( + { a: 3 }, + { + $set: { + a: 5 + } + }, + {} + ) + ).value +); + +// Including { includeResultMetadata: true } option will return the +// modify result. +expectType | null>( + (await coll.findOneAndDelete({ a: 3 }, { includeResultMetadata: true })).value +); +expectType | null>( + ( + await coll.findOneAndReplace( + { a: 3 }, + { a: 5, b: 'new string' }, + { includeResultMetadata: true } + ) + ).value +); +expectType | null>( + (await coll.findOneAndUpdate({ a: 3 }, { $set: { a: 5 } }, { includeResultMetadata: true })).value +); + +// Including { includeResultMetadata: false } option will change the return type +// to the modified document or null. +expectType | null>( + await coll.findOneAndDelete({ a: 3 }, { includeResultMetadata: false }) +); +expectType | null>( + await coll.findOneAndReplace( + { a: 3 }, + { a: 5, b: 'new string' }, + { includeResultMetadata: false } + ) +); +expectType | null>( + await coll.findOneAndUpdate( + { a: 3 }, + { + $set: { + a: 5 + } + }, + { includeResultMetadata: false } + ) +); + // projections do not change the return type - our typing doesn't support this expectType | null>( (await coll.findOneAndDelete({ a: 3 }, { projection: { _id: 0 } })).value diff --git a/test/types/community/transaction.test-d.ts b/test/types/community/transaction.test-d.ts index e9ca1b97ce0..d329b09ab2e 100644 --- a/test/types/community/transaction.test-d.ts +++ b/test/types/community/transaction.test-d.ts @@ -1,5 +1,3 @@ -/* eslint-disable @typescript-eslint/no-non-null-assertion */ - import { type ClientSession, MongoClient, ReadConcern } from '../../mongodb'; // TODO(NODE-3345): Improve these tests to use expect assertions more @@ -84,11 +82,13 @@ const db = client.db(); session.startTransaction(); try { const opts = { session, returnOriginal: false }; - const res = await db - .collection('Account') - .findOneAndUpdate({ name: from }, { $inc: { balance: -amount } }, opts); - const A = res.value!; - if (A.balance < 0) { + const res = ( + await db + .collection('Account') + .findOneAndUpdate({ name: from }, { $inc: { balance: -amount } }, opts) + ).value; + const A = res; + if (A?.balance && A.balance < 0) { // If A would have negative balance, fail and abort the transaction // `session.abortTransaction()` will undo the above `findOneAndUpdate()` throw new Error('Insufficient funds: ' + (A.balance + amount)); @@ -97,7 +97,7 @@ try { const resB = await db .collection('Account') .findOneAndUpdate({ name: to }, { $inc: { balance: amount } }, opts); - const B = resB.value!; + const B = resB; await session.commitTransaction(); session.endSession();