From 747ae74c02019072bdc7898c0c93607eac011f95 Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 26 Jul 2023 10:55:56 -0400 Subject: [PATCH 01/18] feat(NODE-3568)!: switch default --- src/operations/find_and_modify.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/operations/find_and_modify.ts b/src/operations/find_and_modify.ts index 2d97ffb23e2..e8481badffe 100644 --- a/src/operations/find_and_modify.ts +++ b/src/operations/find_and_modify.ts @@ -142,7 +142,7 @@ class FindAndModifyOperation extends CommandOperation { upsert: false }; - options.includeResultMetadata ??= true; + options.includeResultMetadata ??= false; const sort = formatSort(options.sort); if (sort) { From 0ebba1054759efdea2992be3abfb742b0a90f015 Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 26 Jul 2023 15:12:26 -0400 Subject: [PATCH 02/18] test(NODE-3568): update failing tests --- test/integration/crud/crud_api.test.ts | 8 +- test/integration/crud/find.test.js | 309 ++++++++----------- test/tools/unified-spec-runner/operations.ts | 6 +- 3 files changed, 131 insertions(+), 192 deletions(-) diff --git a/test/integration/crud/crud_api.test.ts b/test/integration/crud/crud_api.test.ts index b5f97540cec..8c3c116ba61 100644 --- a/test/integration/crud/crud_api.test.ts +++ b/test/integration/crud/crud_api.test.ts @@ -638,7 +638,7 @@ describe('CRUD API', function () { it('returns the modify result', async function () { const result = await collection.findOneAndDelete( { a: 1 }, - { projection: { b: 1 }, sort: { a: 1 } } + { projection: { b: 1 }, sort: { a: 1 }, includeResultMetadata: true } ); expect(result?.lastErrorObject.n).to.equal(1); expect(result?.value.b).to.equal(1); @@ -685,7 +685,8 @@ describe('CRUD API', function () { projection: { b: 1, c: 1 }, sort: { a: 1 }, returnDocument: ReturnDocument.AFTER, - upsert: true + upsert: true, + includeResultMetadata: true } ); expect(result?.lastErrorObject.n).to.equal(1); @@ -742,7 +743,8 @@ describe('CRUD API', function () { projection: { b: 1, d: 1 }, sort: { a: 1 }, returnDocument: ReturnDocument.AFTER, - upsert: true + upsert: true, + includeResultMetadata: true } ); expect(result?.lastErrorObject.n).to.equal(1); diff --git a/test/integration/crud/find.test.js b/test/integration/crud/find.test.js index 6803ece2cbf..6f801ed2977 100644 --- a/test/integration/crud/find.test.js +++ b/test/integration/crud/find.test.js @@ -23,42 +23,30 @@ describe('Find', function () { 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); - const collection = db.collection('test_find_simple'); - const docs = [{ a: 2 }, { b: 3 }]; + test: async function () { + const configuration = this.configuration; + const client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); + await client.connect(); + this.defer(async () => { + await client.close(); + }); - // Insert some test documents - collection.insert(docs, configuration.writeConcernMax(), err => { - expect(err).to.not.exist; + const db = client.db(configuration.db); + await db.dropCollection('test_find_simple').catch(() => null); + const collection = db.collection('test_find_simple'); + const docs = [{ a: 2 }, { b: 3 }]; - // Ensure correct insertion testing via the cursor and the count function - collection.find().toArray(function (err, documents) { - expect(err).to.not.exist; - test.equal(2, documents.length); + await collection.insert(docs, configuration.writeConcernMax()); - collection.count(function (err, count) { - expect(err).to.not.exist; - test.equal(2, count); + const insertedDocs = await collection.find().toArray(); + expect(insertedDocs).to.have.length(2); - // Fetch values by selection - collection.find({ a: docs[0].a }).toArray(function (err, documents) { - expect(err).to.not.exist; + const docCount = await collection.count(); + expect(docCount).to.equal(2); - test.equal(1, documents.length); - test.equal(docs[0].a, documents[0].a); - // Let's close the db - client.close(done); - }); - }); - }); - }); - }); + const valuesBySelection = await collection.find({ a: docs[0].a }).toArray(); + expect(valuesBySelection).to.have.length(1); + expect(valuesBySelection[0].a).to.deep.equal(docs[0].a); } }); @@ -114,98 +102,61 @@ describe('Find', function () { 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) { - var db = client.db(configuration.db); - var collection = db.collection('test_find_advanced'); - const docs = [{ a: 1 }, { a: 2 }, { b: 3 }]; + test: async function () { + const configuration = this.configuration; + const client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); + await client.connect(); + this.defer(async () => await client.close()); - // Insert some test documents - collection.insert(docs, configuration.writeConcernMax(), function (err) { - expect(err).to.not.exist; + //await client.db(configuration.db).dropDatabase(configuration.db); + const db = client.db(configuration.db); + await db.dropCollection('test_find_advanced').catch(() => null); + const collection = db.collection('test_find_advanced'); + const docs = [{ a: 1 }, { a: 2 }, { b: 3 }]; - // Locate by less than - collection.find({ a: { $lt: 10 } }).toArray(function (err, documents) { - test.equal(2, documents.length); - // Check that the correct documents are returned - var results = []; - // Check that we have all the results we want - documents.forEach(function (doc) { - if (doc.a === 1 || doc.a === 2) results.push(1); - }); - test.equal(2, results.length); + await collection.insert(docs, configuration.writeConcernMax()); - // Locate by greater than - collection.find({ a: { $gt: 1 } }).toArray(function (err, documents) { - test.equal(1, documents.length); - test.equal(2, documents[0].a); + const aLT10Docs = await collection.find({ a: { $lt: 10 } }).toArray(); + expect(aLT10Docs).to.have.length(2); + expect(aLT10Docs.filter(doc => doc.a === 1 || doc.a === 2)).to.have.length(2); - // Locate by less than or equal to - collection.find({ a: { $lte: 1 } }).toArray(function (err, documents) { - test.equal(1, documents.length); - test.equal(1, documents[0].a); + const aGT1 = await collection.find({ a: { $gt: 1 } }).toArray(); + expect(aGT1).to.have.length(1); + expect(aGT1[0].a).to.equal(2); - // Locate by greater than or equal to - collection.find({ a: { $gte: 1 } }).toArray(function (err, documents) { - test.equal(2, documents.length); - // Check that the correct documents are returned - var results = []; - // Check that we have all the results we want - documents.forEach(function (doc) { - if (doc.a === 1 || doc.a === 2) results.push(1); - }); - test.equal(2, results.length); - - // Locate by between - collection.find({ a: { $gt: 1, $lt: 3 } }).toArray(function (err, documents) { - test.equal(1, documents.length); - test.equal(2, documents[0].a); - - // Locate in clause - collection.find({ a: { $in: [1, 2] } }).toArray(function (err, documents) { - test.equal(2, documents.length); - // Check that the correct documents are returned - var results = []; - // Check that we have all the results we want - documents.forEach(function (doc) { - if (doc.a === 1 || doc.a === 2) results.push(1); - }); - test.equal(2, results.length); + const aLTE1 = await collection.find({ a: { $lte: 1 } }).toArray(); + expect(aLTE1).to.have.length(1); + expect(aLTE1[0].a).to.equal(1); - // Locate in _id clause - collection - .find({ _id: { $in: [docs[0]['_id'], docs[1]['_id']] } }) - .toArray(function (err, documents) { - test.equal(2, documents.length); - // Check that the correct documents are returned - var results = []; - // Check that we have all the results we want - documents.forEach(function (doc) { - if (doc.a === 1 || doc.a === 2) results.push(1); - }); - test.equal(2, results.length); - // Let's close the db - client.close(done); - }); - }); - }); - }); - }); - }); - }); - }); - }); + const aGTE1 = await collection.find({ a: { $gte: 1 } }).toArray(); + expect(aGTE1).to.have.length(2); + + expect(aGTE1.filter(doc => doc.a === 1 || doc.a === 2)).to.have.length(2); + + const aGT1LT3 = await collection.find({ a: { $gt: 1, $lt: 3 } }).toArray(); + expect(aGT1LT3).to.have.length(1); + expect(aGT1LT3[0].a).to.equal(2); + + const aIN = await collection.find({ a: { $in: [1, 2] } }).toArray(); + expect(aIN).to.have.length(2); + + expect(aIN.filter(doc => doc.a === 1 || doc.a === 2)).to.have.length(2); + + const byID = await collection + .find({ _id: { $in: [docs[0]['_id'], docs[1]['_id']] } }) + .toArray(); + expect(byID).to.have.length(2); + expect(byID.filter(doc => doc.a === 1 || doc.a === 2)).to.have.length(2); } }); - /** * Test sorting of results */ it('shouldCorrectlyPerformFindWithSort', { metadata: { - requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] } + requires: { + topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] + } }, test: function (done) { @@ -784,35 +735,30 @@ describe('Find', function () { /** * Test findOneAndUpdate a document with fields */ - it('shouldCorrectlyfindOneAndUpdateDocumentAndReturnSelectedFieldsOnly', { + it('shouldCorrectlyFindOneAndUpdateDocumentAndReturnSelectedFieldsOnly', { 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) { - var db = client.db(configuration.db); - db.createCollection('test_find_and_modify_a_document_2', function (err, collection) { - // 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, projection: { a: 1 } }, - function (err, updated_doc) { - expect(Object.keys(updated_doc.value).length).to.equal(2); - expect(updated_doc.value.a).to.equal(1); - client.close(done); - } - ); - }); - }); + test: async function () { + const configuration = this.configuration; + const client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); + await client.connect(); + this.defer(async () => { + await client.close(); }); + const db = client.db(configuration.db); + const coll = db.collection('test_find_and_modify_a_document_2'); + await coll.insert({ a: 1, b: 2 }, configuration.writeConcernMax()); + + const updatedDoc = await coll.findOneAndUpdate( + { a: 1 }, + { $set: { b: 3 } }, + { returnDocument: ReturnDocument.AFTER, projection: { a: 1 } } + ); + + expect(Object.keys(updatedDoc).length).to.equal(2); + expect(updatedDoc.a).to.equal(1); } }); @@ -866,28 +812,24 @@ describe('Find', function () { 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) { - var db = client.db(configuration.db); - db.createCollection( - 'AttemptTofindOneAndUpdateNonExistingDocument', - function (err, collection) { - // Let's modify the document in place - collection.findOneAndUpdate( - { name: 'test1' }, - { $set: { name: 'test2' } }, - {}, - function (err, updated_doc) { - expect(updated_doc.value).to.not.exist; - test.ok(err == null || err.errmsg.match('No matching object found')); - client.close(done); - } - ); - } - ); + test: async function () { + const configuration = this.configuration; + const client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); + await client.connect(); + this.defer(async () => { + await client.close(); }); + + const db = client.db(configuration.db); + const coll = db.collection('AttemptTofindOneAndUpdateNonExistingDocument'); + + const updatedDoc = await coll.findOneAndUpdate( + { name: 'test1' }, + { $set: { name: 'test2' } }, + {} + ); + + expect(updatedDoc).to.be.null; } }); @@ -1073,39 +1015,34 @@ describe('Find', function () { 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) { - var db = client.db(configuration.db); - db.createCollection( - 'Should_correctly_return_new_modified_document', - function (err, collection) { - var id = new ObjectId(); - var doc = { _id: id, a: 1, b: 1, c: { a: 1, b: 1 } }; + test: async function () { + const configuration = this.configuration; + const client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); + await client.connect(); + this.defer(async () => { + await client.close(); + }); - collection.insert(doc, configuration.writeConcernMax(), function (err) { - expect(err).to.not.exist; + const db = client.db(configuration.db); + const col = await db.collection('Should_correctly_return_new_modified_document'); - // Find and modify returning the new object - collection.findOneAndUpdate( - { _id: id }, - { $set: { 'c.c': 100 } }, - { returnDocument: ReturnDocument.AFTER }, - function (err, item) { - 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); - } - ); - }); - } - ); - }); + const id = new ObjectId(); + const doc = { _id: id, a: 1, b: 1, c: { a: 1, b: 1 } }; + + await col.insert(doc, configuration.writeConcernMax()); + + const item = await col.findOneAndUpdate( + { _id: id }, + { $set: { 'c.c': 100 } }, + { returnDocument: ReturnDocument.AFTER } + ); + + expect(item._id.toString()).to.equal(doc._id.toString()); + expect(item.a).to.equal(doc.a); + expect(item.b).to.equal(doc.b); + expect(item.c.a).to.equal(doc.c.a); + expect(item.c.b).to.equal(doc.c.b); + expect(item.c.c).to.equal(100); } }); diff --git a/test/tools/unified-spec-runner/operations.ts b/test/tools/unified-spec-runner/operations.ts index 304b8cae06d..16a07a9bbb6 100644 --- a/test/tools/unified-spec-runner/operations.ts +++ b/test/tools/unified-spec-runner/operations.ts @@ -317,19 +317,19 @@ operations.set('findOne', async ({ entities, operation }) => { operations.set('findOneAndReplace', async ({ entities, operation }) => { const collection = entities.getEntity('collection', operation.object); const { filter, replacement, ...opts } = operation.arguments!; - return (await collection.findOneAndReplace(filter, replacement, translateOptions(opts))).value; + return collection.findOneAndReplace(filter, replacement, translateOptions(opts)); }); operations.set('findOneAndUpdate', async ({ entities, operation }) => { const collection = entities.getEntity('collection', operation.object); const { filter, update, ...opts } = operation.arguments!; - return (await collection.findOneAndUpdate(filter, update, translateOptions(opts))).value; + return collection.findOneAndUpdate(filter, update, translateOptions(opts)); }); operations.set('findOneAndDelete', async ({ entities, operation }) => { const collection = entities.getEntity('collection', operation.object); const { filter, ...opts } = operation.arguments!; - return (await collection.findOneAndDelete(filter, opts)).value; + return collection.findOneAndDelete(filter, opts); }); operations.set('failPoint', async ({ entities, operation }) => { From f95e410ae20cfb6c2f34a3438d649db7366f3e46 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 27 Jul 2023 15:37:27 -0400 Subject: [PATCH 03/18] test(NODE-3568): use metadata in explain tests --- test/integration/crud/explain.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/integration/crud/explain.test.ts b/test/integration/crud/explain.test.ts index d00caff025d..9961fd1a7ef 100644 --- a/test/integration/crud/explain.test.ts +++ b/test/integration/crud/explain.test.ts @@ -42,7 +42,7 @@ describe('CRUD API explain option', function () { { name: 'findOneAndDelete', op: async (explain: boolean | string) => - await collection.findOneAndDelete({ a: 1 }, { explain }) + await collection.findOneAndDelete({ a: 1 }, { explain, includeResultMetadata: true }) }, { name: 'findOne', @@ -52,7 +52,11 @@ describe('CRUD API explain option', function () { { name: 'findOneAndReplace', op: async (explain: boolean | string) => - await collection.findOneAndReplace({ a: 1 }, { a: 2 }, { explain }) + await collection.findOneAndReplace( + { a: 1 }, + { a: 2 }, + { explain, includeResultMetadata: true } + ) }, { name: 'aggregate', From 94213881e3504356dce6d76548ad5231387ca4dc Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 27 Jul 2023 15:57:19 -0400 Subject: [PATCH 04/18] test(NODE-3568): fix test --- test/integration/crud/find_and_modify.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/integration/crud/find_and_modify.test.ts b/test/integration/crud/find_and_modify.test.ts index 81312425034..a7c6faed409 100644 --- a/test/integration/crud/find_and_modify.test.ts +++ b/test/integration/crud/find_and_modify.test.ts @@ -25,7 +25,7 @@ describe('Collection (#findOneAnd...)', function () { }); it('returns the raw result', async function () { - const result = await collection.findOneAndDelete({ a: 1 }); + const result = await collection.findOneAndDelete({ a: 1 }, { includeResultMetadata: true }); expect(result.value.b).to.equal(1); }); }); @@ -171,7 +171,7 @@ describe('Collection (#findOneAnd...)', function () { }); it('returns the raw result', async function () { - const result = await collection.findOneAndUpdate({ a: 1 }, { $set: { a: 1 } }); + const result = await collection.findOneAndUpdate({ a: 1 }, { $set: { a: 1 } }, { includeResultMetadata: true }); expect(result.value.b).to.equal(1); }); }); @@ -349,7 +349,7 @@ describe('Collection (#findOneAnd...)', function () { }); it('returns the raw result', async function () { - const result = await collection.findOneAndReplace({ a: 1 }, { a: 1 }); + const result = await collection.findOneAndReplace({ a: 1 }, { a: 1 }, { includeResultMetadata: true }); expect(result.value.b).to.equal(1); }); }); From bd70e9313d19a3af1e0da757f5429c9c7c8ac08e Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 27 Jul 2023 15:58:15 -0400 Subject: [PATCH 05/18] style(NODE-3568): eslint --- test/integration/crud/find_and_modify.test.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/integration/crud/find_and_modify.test.ts b/test/integration/crud/find_and_modify.test.ts index a7c6faed409..2dfdeb85fe7 100644 --- a/test/integration/crud/find_and_modify.test.ts +++ b/test/integration/crud/find_and_modify.test.ts @@ -171,7 +171,11 @@ describe('Collection (#findOneAnd...)', function () { }); it('returns the raw result', async function () { - const result = await collection.findOneAndUpdate({ a: 1 }, { $set: { a: 1 } }, { includeResultMetadata: true }); + const result = await collection.findOneAndUpdate( + { a: 1 }, + { $set: { a: 1 } }, + { includeResultMetadata: true } + ); expect(result.value.b).to.equal(1); }); }); @@ -349,7 +353,11 @@ describe('Collection (#findOneAnd...)', function () { }); it('returns the raw result', async function () { - const result = await collection.findOneAndReplace({ a: 1 }, { a: 1 }, { includeResultMetadata: true }); + const result = await collection.findOneAndReplace( + { a: 1 }, + { a: 1 }, + { includeResultMetadata: true } + ); expect(result.value.b).to.equal(1); }); }); From 4c1dd450cdeefc3d5358057e839ece65d97aa6a7 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 27 Jul 2023 16:43:11 -0400 Subject: [PATCH 06/18] test(NODE-3568): update test files --- test/integration/crud/find.test.js | 14 +++++++---- test/integration/crud/find_and_modify.test.ts | 23 +++++++------------ test/integration/crud/insert.test.js | 3 ++- .../node-specific/client_encryption.test.ts | 5 +++- 4 files changed, 23 insertions(+), 22 deletions(-) diff --git a/test/integration/crud/find.test.js b/test/integration/crud/find.test.js index 6f801ed2977..8f76025feab 100644 --- a/test/integration/crud/find.test.js +++ b/test/integration/crud/find.test.js @@ -1222,7 +1222,7 @@ describe('Find', function () { collection.findOneAndUpdate( { a: 1 }, { $set: { b: 3 } }, - { returnDocument: ReturnDocument.AFTER }, + { returnDocument: ReturnDocument.AFTER, includeResultMetadata: true }, function (err, updated_doc) { expect(err).to.not.exist; expect(updated_doc.value).to.not.exist; @@ -1283,7 +1283,7 @@ describe('Find', function () { 'transactions.id': { $ne: transaction.transactionId } }, { $push: { transactions: transaction } }, - { returnDocument: ReturnDocument.AFTER, safe: true }, + { returnDocument: ReturnDocument.AFTER, safe: true, includeResultMetadata: true }, function (err) { expect(err).to.not.exist; client.close(done); @@ -1369,7 +1369,11 @@ describe('Find', function () { function (err, collection) { var q = { x: 1 }; var set = { y: 2, _id: new ObjectId() }; - var opts = { returnDocument: ReturnDocument.AFTER, upsert: true }; + var opts = { + returnDocument: ReturnDocument.AFTER, + upsert: true, + includeResultMetadata: true + }; // Original doc var doc = { _id: new ObjectId(), x: 1 }; @@ -1521,7 +1525,7 @@ describe('Find', function () { collection.findOneAndUpdate( { _id: id }, { $set: { login: 'user1' } }, - {}, + { includeResultMetadata: true }, function (err) { test.ok(err !== null); p_client.close(done); @@ -2095,7 +2099,7 @@ describe('Find', function () { collection.findOneAndUpdate( { a: 1 }, { $set: { b: 3 } }, - { returnDocument: ReturnDocument.AFTER }, + { returnDocument: ReturnDocument.AFTER, includeResultMetadata: true }, function (err, updated_doc) { expect(updated_doc.value.a).to.equal(1); expect(updated_doc.value.b).to.equal(3); diff --git a/test/integration/crud/find_and_modify.test.ts b/test/integration/crud/find_and_modify.test.ts index 2dfdeb85fe7..8d1dd976acf 100644 --- a/test/integration/crud/find_and_modify.test.ts +++ b/test/integration/crud/find_and_modify.test.ts @@ -9,7 +9,7 @@ describe('Collection (#findOneAnd...)', function () { }); describe('#findOneAndDelete', function () { - context('when no options are passed', function () { + context('when passing includeResultMetadata: true', function () { let client; let collection; @@ -30,7 +30,7 @@ describe('Collection (#findOneAnd...)', function () { }); }); - context('when passing includeResultMetadata: false', function () { + context('when no options are passed', function () { let client; let collection; @@ -47,10 +47,7 @@ describe('Collection (#findOneAnd...)', function () { context('when there is a match', function () { it('returns the deleted document', async function () { - const result = await collection.findOneAndDelete( - { a: 1 }, - { includeResultMetadata: false } - ); + const result = await collection.findOneAndDelete({ a: 1 }); expect(result.b).to.equal(1); }); }); @@ -155,7 +152,7 @@ describe('Collection (#findOneAnd...)', function () { }); describe('#findOneAndUpdate', function () { - context('when no options are passed', function () { + context('when passing includeResultMetadata: true', function () { let client; let collection; @@ -180,7 +177,7 @@ describe('Collection (#findOneAnd...)', function () { }); }); - context('when passing includeResultMetadata: false', function () { + context('when no options are passed', function () { let client; let collection; @@ -337,7 +334,7 @@ describe('Collection (#findOneAnd...)', function () { }); describe('#findOneAndReplace', function () { - context('when no options are passed', function () { + context('when passing includeResultMetadata: true', function () { let client; let collection; @@ -362,7 +359,7 @@ describe('Collection (#findOneAnd...)', function () { }); }); - context('when passing includeResultMetadata: false', function () { + context('when no options are passed', function () { let client; let collection; @@ -379,11 +376,7 @@ describe('Collection (#findOneAnd...)', function () { 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 } - ); + const result = await collection.findOneAndReplace({ a: 1 }, { a: 1 }); expect(result.b).to.equal(1); }); }); diff --git a/test/integration/crud/insert.test.js b/test/integration/crud/insert.test.js index b8b0305b703..10a5403b827 100644 --- a/test/integration/crud/insert.test.js +++ b/test/integration/crud/insert.test.js @@ -883,7 +883,8 @@ describe('crud - insert', function () { { returnDocument: ReturnDocument.AFTER, safe: true, - serializeFunctions: true + serializeFunctions: true, + includeResultMetadata: true }, function (err, result) { test.ok(result.value.f._bsontype === 'Code'); diff --git a/test/integration/node-specific/client_encryption.test.ts b/test/integration/node-specific/client_encryption.test.ts index 561fdde95d5..c01fe0d0c32 100644 --- a/test/integration/node-specific/client_encryption.test.ts +++ b/test/integration/node-specific/client_encryption.test.ts @@ -141,7 +141,10 @@ describe('ClientEncryption integration tests', function () { // Remove and re-insert with a fixed UUID to guarantee consistent output const doc = ( - await keyVaultColl.findOneAndDelete({ _id: dataKey }, { writeConcern: { w: 'majority' } }) + await keyVaultColl.findOneAndDelete( + { _id: dataKey }, + { writeConcern: { w: 'majority' }, includeResultMetadata: true } + ) ).value; doc._id = new Binary(Buffer.alloc(16), 4); await keyVaultColl.insertOne(doc, { writeConcern: { w: 'majority' } }); From b128e90e31273296aff24add252d18a94165deb0 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 27 Jul 2023 16:45:27 -0400 Subject: [PATCH 07/18] test(NODE-3568): update test to reflect test name --- test/integration/crud/find_and_modify.test.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/integration/crud/find_and_modify.test.ts b/test/integration/crud/find_and_modify.test.ts index 8d1dd976acf..db28381732b 100644 --- a/test/integration/crud/find_and_modify.test.ts +++ b/test/integration/crud/find_and_modify.test.ts @@ -56,7 +56,6 @@ describe('Collection (#findOneAnd...)', function () { it('returns null', async function () { const result = await collection.findOneAndDelete( { a: 2 }, - { includeResultMetadata: false } ); expect(result).to.equal(null); }); @@ -197,7 +196,6 @@ describe('Collection (#findOneAnd...)', function () { const result = await collection.findOneAndUpdate( { a: 1 }, { $set: { a: 1 } }, - { includeResultMetadata: false } ); expect(result.b).to.equal(1); }); @@ -208,7 +206,6 @@ describe('Collection (#findOneAnd...)', function () { const result = await collection.findOneAndUpdate( { a: 2 }, { $set: { a: 1 } }, - { includeResultMetadata: false } ); expect(result).to.equal(null); }); @@ -386,7 +383,6 @@ describe('Collection (#findOneAnd...)', function () { const result = await collection.findOneAndReplace( { a: 2 }, { a: 1 }, - { includeResultMetadata: false } ); expect(result).to.equal(null); }); From 1fefda2199f53ad549639a764e32bf2a984235e6 Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 28 Jul 2023 14:54:51 -0400 Subject: [PATCH 08/18] test(NODE-3568): more fixed tests --- test/integration/crud/find.test.js | 2 +- test/integration/crud/find_and_modify.test.ts | 25 +++++++------------ 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/test/integration/crud/find.test.js b/test/integration/crud/find.test.js index 8f76025feab..7212442139d 100644 --- a/test/integration/crud/find.test.js +++ b/test/integration/crud/find.test.js @@ -941,7 +941,7 @@ describe('Find', function () { collection.findOneAndUpdate( { a: 2 }, { $set: { b: 3 } }, - { returnDocument: ReturnDocument.AFTER }, + { returnDocument: ReturnDocument.AFTER, includeResultMetadata: true }, function (err, result) { expect(result.value.a).to.equal(2); expect(result.value.b).to.equal(3); diff --git a/test/integration/crud/find_and_modify.test.ts b/test/integration/crud/find_and_modify.test.ts index db28381732b..16764f020ea 100644 --- a/test/integration/crud/find_and_modify.test.ts +++ b/test/integration/crud/find_and_modify.test.ts @@ -54,9 +54,7 @@ describe('Collection (#findOneAnd...)', function () { context('when there is no match', function () { it('returns null', async function () { - const result = await collection.findOneAndDelete( - { a: 2 }, - ); + const result = await collection.findOneAndDelete({ a: 2 }); expect(result).to.equal(null); }); }); @@ -193,20 +191,14 @@ describe('Collection (#findOneAnd...)', function () { context('when there is a match', function () { it('returns the modified document', async function () { - const result = await collection.findOneAndUpdate( - { a: 1 }, - { $set: { a: 1 } }, - ); + const result = await collection.findOneAndUpdate({ a: 1 }, { $set: { a: 1 } }); 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 } }, - ); + const result = await collection.findOneAndUpdate({ a: 2 }, { $set: { a: 1 } }); expect(result).to.equal(null); }); }); @@ -265,7 +257,11 @@ describe('Collection (#findOneAnd...)', function () { }); it('returns the raw result', async function () { - const result = await collection.findOneAndUpdate({ a: 1 }, { $set: { a: 1 } }); + const result = await collection.findOneAndUpdate( + { a: 1 }, + { $set: { a: 1 } }, + { includeResultMetadata: true } + ); expect(result.value.b).to.equal(1); }); } @@ -380,10 +376,7 @@ describe('Collection (#findOneAnd...)', function () { context('when there is no match', function () { it('returns null', async function () { - const result = await collection.findOneAndReplace( - { a: 2 }, - { a: 1 }, - ); + const result = await collection.findOneAndReplace({ a: 2 }, { a: 1 }); expect(result).to.equal(null); }); }); From b59ecc691c13c9d9c214028d5f196623f3e2426d Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 28 Jul 2023 15:25:23 -0400 Subject: [PATCH 09/18] test(NODE-3568): fix client encryption tests --- src/client-side-encryption/client_encryption.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client-side-encryption/client_encryption.ts b/src/client-side-encryption/client_encryption.ts index 5d06134c226..391659d8740 100644 --- a/src/client-side-encryption/client_encryption.ts +++ b/src/client-side-encryption/client_encryption.ts @@ -444,7 +444,7 @@ export class ClientEncryption implements StateMachineExecutable { this._keyVaultNamespace ); - const { value } = await this._keyVaultClient + const value = await this._keyVaultClient .db(dbName) .collection(collectionName) .findOneAndUpdate( @@ -506,7 +506,7 @@ export class ClientEncryption implements StateMachineExecutable { } } ]; - const { value } = await this._keyVaultClient + const value = await this._keyVaultClient .db(dbName) .collection(collectionName) .findOneAndUpdate({ _id }, pipeline, { From aa9002d46ef96ee6f8f9bb96b634fa96ccaa1768 Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 28 Jul 2023 15:37:51 -0400 Subject: [PATCH 10/18] docs(NODE-3568): Update API docs --- src/operations/find_and_modify.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/operations/find_and_modify.ts b/src/operations/find_and_modify.ts index e8481badffe..0e0d786be3d 100644 --- a/src/operations/find_and_modify.ts +++ b/src/operations/find_and_modify.ts @@ -30,8 +30,7 @@ export interface FindOneAndDeleteOptions extends CommandOperationOptions { /** Map of parameter names and values that can be accessed using $$var (requires MongoDB 5.0). */ let?: Document; /** - * Return the ModifyResult instead of the modified document. Defaults to true - * but will default to false in the next major version. + * Return the ModifyResult instead of the modified document. Defaults to false */ includeResultMetadata?: boolean; } From c8c5479765bcaf565c3df85316f6aae1208d9ff1 Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 28 Jul 2023 16:56:47 -0400 Subject: [PATCH 11/18] test(NODE-3568): fix leftofver tests --- .../node-specific/operation_examples.test.ts | 54 ++++++++++++------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/test/integration/node-specific/operation_examples.test.ts b/test/integration/node-specific/operation_examples.test.ts index 8c06adaca72..6437b121272 100644 --- a/test/integration/node-specific/operation_examples.test.ts +++ b/test/integration/node-specific/operation_examples.test.ts @@ -936,7 +936,7 @@ describe('Operations', function () { return collection.findOneAndUpdate( { a: 1 }, { $set: { b1: 1 } }, - { returnDocument: ReturnDocument.AFTER } + { returnDocument: ReturnDocument.AFTER, includeResultMetadata: true } ); }) .then(function (doc) { @@ -945,7 +945,11 @@ describe('Operations', function () { // Simple findAndModify command returning the new document and // removing it at the same time - return collection.findOneAndUpdate({ b: 1 }, { $set: { b: 2 } }, { remove: true }); + return collection.findOneAndUpdate( + { b: 1 }, + { $set: { b: 2 } }, + { remove: true, includeResultMetadata: true } + ); }) .then(function (doc) { expect(doc).to.exist; @@ -961,7 +965,12 @@ describe('Operations', function () { return collection.findOneAndUpdate( { d: 1 }, { $set: { d: 1, f: 1 } }, - { returnDocument: ReturnDocument.AFTER, upsert: true, writeConcern: { w: 1 } } + { + returnDocument: ReturnDocument.AFTER, + upsert: true, + writeConcern: { w: 1 }, + includeResultMetadata: true + } ); }) .then(function (doc) { @@ -1011,7 +1020,9 @@ describe('Operations', function () { // Simple findAndModify command returning the old document and // removing it at the same time - return collection.findOneAndDelete({ b: 1 }, [['b', 1]]); + return collection.findOneAndDelete({ b: 1 }, [['b', 1]], { + includeResultMetadata: true + }); }) .then(function (doc) { expect(doc.value.b).to.equal(1); @@ -1655,7 +1666,7 @@ describe('Operations', function () { /* eslint-disable */ - return client.connect().then(function (client) { + return client.connect().then(function(client) { var db = client.db(configuration.db); // LINE var MongoClient = require('mongodb').MongoClient, // LINE test = require('assert'); @@ -1673,7 +1684,7 @@ describe('Operations', function () { db.createCollection('test_rename_collection_with_promise'), db.createCollection('test_rename_collection2_with_promise') ]) - .then(function (collections) { + .then(function(collections) { collection1 = collections[0]; collection2 = collections[1]; @@ -1681,7 +1692,7 @@ describe('Operations', function () { // Attemp to rename a collection to a number try { - collection1.rename(5, function (err, collection) {}); + collection1.rename(5, function(err, collection) { }); } catch (err) { expect(err instanceof Error).to.exist; expect(err.message).to.equal('Collection name must be a String'); @@ -1689,7 +1700,7 @@ describe('Operations', function () { // Attemp to rename a collection to an empty string try { - collection1.rename('', function (err, collection) {}); + collection1.rename('', function(err, collection) { }); } catch (err) { expect(err instanceof Error).to.exist; expect(err.message).to.equal('Collection names cannot be empty'); @@ -1697,7 +1708,7 @@ describe('Operations', function () { // Attemp to rename a collection to an illegal name including the character $ try { - collection1.rename('te$t', function (err, collection) {}); + collection1.rename('te$t', function(err, collection) { }); } catch (err) { expect(err instanceof Error).to.exist; expect(err.message).to.equal("Collection names must not contain '$'"); @@ -1705,7 +1716,7 @@ describe('Operations', function () { // Attemp to rename a collection to an illegal name starting with the character . try { - collection1.rename('.test', function (err, collection) {}); + collection1.rename('.test', function(err, collection) { }); } catch (err) { expect(err instanceof Error).to.exist; expect(err.message).to.equal("Collection names must not start or end with '.'"); @@ -1713,7 +1724,7 @@ describe('Operations', function () { // Attemp to rename a collection to an illegal name ending with the character . try { - collection1.rename('test.', function (err, collection) {}); + collection1.rename('test.', function(err, collection) { }); } catch (err) { expect(err instanceof Error).to.exist; expect(err.message).to.equal("Collection names must not start or end with '.'"); @@ -1721,7 +1732,7 @@ describe('Operations', function () { // Attemp to rename a collection to an illegal name with an empty middle name try { - collection1.rename('tes..t', function (err, collection) {}); + collection1.rename('tes..t', function(err, collection) { }); } catch (err) { expect(err.message).to.equal('Collection names cannot be empty'); } @@ -1729,13 +1740,13 @@ describe('Operations', function () { // Insert a couple of documents return collection1.insertMany([{ x: 1 }, { x: 2 }], configuration.writeConcernMax()); }) - .then(function (docs) { + .then(function(docs) { expect(docs).to.exist; // Attemp to rename the first collection to the second one, this will fail return collection1.rename('test_rename_collection2_with_promise'); }) - .catch(function (err) { + .catch(function(err) { expect(err instanceof Error).to.exist; expect(err.message.length > 0).to.exist; @@ -1743,13 +1754,13 @@ describe('Operations', function () { // this will be successful return collection1.rename('test_rename_collection3_with_promise'); }) - .then(function (collection2) { + .then(function(collection2) { expect(collection2.collectionName).to.equal('test_rename_collection3_with_promise'); // Ensure that the collection is pointing to the new one return collection2.count(); }) - .then(function (count) { + .then(function(count) { expect(count).to.equal(2); }) .then( @@ -4000,7 +4011,10 @@ describe('Operations', function () { .insertMany([{ a: 1, b: 1 }], { writeConcern: { w: 1 } }) .then(function (r) { expect(r).property('insertedCount').to.equal(1); - return col.findOneAndDelete({ a: 1 }, { projection: { b: 1 }, sort: { a: 1 } }); + return col.findOneAndDelete( + { a: 1 }, + { projection: { b: 1 }, sort: { a: 1 }, includeResultMetadata: true } + ); }) .then(function (r) { expect(r.lastErrorObject.n).to.equal(1); @@ -4051,7 +4065,8 @@ describe('Operations', function () { projection: { b: 1, c: 1 }, sort: { a: 1 }, returnDocument: ReturnDocument.AFTER, - upsert: true + upsert: true, + includeResultMetadata: true } ) .then(function (r) { @@ -4106,7 +4121,8 @@ describe('Operations', function () { projection: { b: 1, d: 1 }, sort: { a: 1 }, returnDocument: ReturnDocument.AFTER, - upsert: true + upsert: true, + includeResultMetadata: true } ); }) From 5588738bcc7cf6620d55e31a76756afe32bb03bd Mon Sep 17 00:00:00 2001 From: Warren James Date: Mon, 31 Jul 2023 11:40:06 -0400 Subject: [PATCH 12/18] test(NODE-3568): fix example test --- .../integration/node-specific/operation_examples.test.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/integration/node-specific/operation_examples.test.ts b/test/integration/node-specific/operation_examples.test.ts index 6437b121272..92bec46ac9a 100644 --- a/test/integration/node-specific/operation_examples.test.ts +++ b/test/integration/node-specific/operation_examples.test.ts @@ -1020,9 +1020,12 @@ describe('Operations', function () { // Simple findAndModify command returning the old document and // removing it at the same time - return collection.findOneAndDelete({ b: 1 }, [['b', 1]], { - includeResultMetadata: true - }); + return collection.findOneAndDelete( + { b: 1 }, + { + includeResultMetadata: true + } + ); }) .then(function (doc) { expect(doc.value.b).to.equal(1); From ce96e9eb31cf918b6673228d785d01b6a862ec75 Mon Sep 17 00:00:00 2001 From: Warren James Date: Tue, 1 Aug 2023 13:44:38 -0400 Subject: [PATCH 13/18] fix(NODE-3568): review feedback --- test/integration/crud/find.test.js | 2 +- .../node-specific/operation_examples.test.ts | 183 +++++++----------- 2 files changed, 74 insertions(+), 111 deletions(-) diff --git a/test/integration/crud/find.test.js b/test/integration/crud/find.test.js index 7212442139d..c930a86a4ea 100644 --- a/test/integration/crud/find.test.js +++ b/test/integration/crud/find.test.js @@ -735,7 +735,7 @@ describe('Find', function () { /** * Test findOneAndUpdate a document with fields */ - it('shouldCorrectlyFindOneAndUpdateDocumentAndReturnSelectedFieldsOnly', { + it('returns selected fields only for findOneAndUpdate', { metadata: { requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] } }, diff --git a/test/integration/node-specific/operation_examples.test.ts b/test/integration/node-specific/operation_examples.test.ts index 92bec46ac9a..179e7104a3b 100644 --- a/test/integration/node-specific/operation_examples.test.ts +++ b/test/integration/node-specific/operation_examples.test.ts @@ -1661,121 +1661,84 @@ describe('Operations', function () { requires: { topology: ['single'] } }, - test: function () { + test: async function () { const configuration = this.configuration; - const client = configuration.newClient(configuration.writeConcernMax(), { + const client: MongoClient = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); + // LINE var MongoClient = require('mongodb').MongoClient, + // LINE test = require('assert'); + // LINE const client = new MongoClient('mongodb://localhost:27017/test'); + // LINE client.connect().then(() => { + // LINE var db = client.db('test); + // REPLACE configuration.writeConcernMax() WITH {w:1} + // REMOVE-LINE done(); + // BEGIN + // Open a couple of collections - /* eslint-disable */ - - return client.connect().then(function(client) { - var db = client.db(configuration.db); - // LINE var MongoClient = require('mongodb').MongoClient, - // LINE test = require('assert'); - // LINE const client = new MongoClient('mongodb://localhost:27017/test'); - // LINE client.connect().then(() => { - // LINE var db = client.db('test); - // REPLACE configuration.writeConcernMax() WITH {w:1} - // REMOVE-LINE done(); - // BEGIN - // Open a couple of collections - - var collection1, collection2; - - return Promise.all([ - db.createCollection('test_rename_collection_with_promise'), - db.createCollection('test_rename_collection2_with_promise') - ]) - .then(function(collections) { - collection1 = collections[0]; - collection2 = collections[1]; - - expect(collection2).to.exist; - - // Attemp to rename a collection to a number - try { - collection1.rename(5, function(err, collection) { }); - } catch (err) { - expect(err instanceof Error).to.exist; - expect(err.message).to.equal('Collection name must be a String'); - } - - // Attemp to rename a collection to an empty string - try { - collection1.rename('', function(err, collection) { }); - } catch (err) { - expect(err instanceof Error).to.exist; - expect(err.message).to.equal('Collection names cannot be empty'); - } - - // Attemp to rename a collection to an illegal name including the character $ - try { - collection1.rename('te$t', function(err, collection) { }); - } catch (err) { - expect(err instanceof Error).to.exist; - expect(err.message).to.equal("Collection names must not contain '$'"); - } - - // Attemp to rename a collection to an illegal name starting with the character . - try { - collection1.rename('.test', function(err, collection) { }); - } catch (err) { - expect(err instanceof Error).to.exist; - expect(err.message).to.equal("Collection names must not start or end with '.'"); - } - - // Attemp to rename a collection to an illegal name ending with the character . - try { - collection1.rename('test.', function(err, collection) { }); - } catch (err) { - expect(err instanceof Error).to.exist; - expect(err.message).to.equal("Collection names must not start or end with '.'"); - } - - // Attemp to rename a collection to an illegal name with an empty middle name - try { - collection1.rename('tes..t', function(err, collection) { }); - } catch (err) { - expect(err.message).to.equal('Collection names cannot be empty'); - } - - // Insert a couple of documents - return collection1.insertMany([{ x: 1 }, { x: 2 }], configuration.writeConcernMax()); - }) - .then(function(docs) { - expect(docs).to.exist; - - // Attemp to rename the first collection to the second one, this will fail - return collection1.rename('test_rename_collection2_with_promise'); - }) - .catch(function(err) { - expect(err instanceof Error).to.exist; - expect(err.message.length > 0).to.exist; - - // Attemp to rename the first collection to a name that does not exist - // this will be successful - return collection1.rename('test_rename_collection3_with_promise'); - }) - .then(function(collection2) { - expect(collection2.collectionName).to.equal('test_rename_collection3_with_promise'); - - // Ensure that the collection is pointing to the new one - return collection2.count(); - }) - .then(function(count) { - expect(count).to.equal(2); - }) - .then( - () => client.close(), - e => { - client.close(); - throw e; - } - ); - }); + await client.connect(); + this.defer(async () => await client.close()); + const db = client.db(configuration.db); + /* es-lint-disable */ + let [collection1] = await Promise.all([ + db.createCollection('test_rename_collection_with_promise'), + db.createCollection('test_rename_collection2_with_promise') + ]); + + // Attempt to rename a collection to a number + // @ts-expect-error need to test that it will fail on number inputs + let err = await collection1.rename(5).catch(e => e); + expect(err).to.be.instanceOf(Error); + expect(err).to.have.property('message', 'Collection name must be a String'); + + // Attempt to rename a collection to an empty string + err = await collection1.rename('').catch(e => e); + expect(err).to.be.instanceOf(Error); + expect(err).to.have.property('message', 'Collection names cannot be empty'); + + // Attemp to rename a collection to an illegal name including the character $ + err = await collection1.rename('te$t').catch(e => e); + expect(err).to.be.instanceOf(Error); + expect(err).to.have.property('message', "Collection names must not contain '$'"); + + // Attempt to rename a collection to an illegal name starting with the character . + err = await collection1.rename('.test').catch(e => e); + expect(err).to.be.instanceOf(Error); + expect(err).to.have.property('message', "Collection names must not start or end with '.'"); + // Attempt to rename a collection to an illegal name ending with the character . + err = await collection1.rename('test.').catch(e => e); + expect(err).to.be.instanceOf(Error); + expect(err).to.have.property('message', "Collection names must not start or end with '.'"); + + // Attempt to rename a collection to an illegal name with an empty middle name + err = await collection1.rename('tes..t').catch(e => e); + expect(err).to.be.instanceOf(Error); + expect(err).to.have.property('message', 'Collection names cannot be empty'); + + // Attempt to rename a collection to an illegal name with an empty middle name + err = await collection1.rename('tes..t').catch(e => e); + expect(err).to.be.instanceOf(Error); + expect(err).to.have.property('message', 'Collection names cannot be empty'); + + // Insert a couple of documents + await collection1.insertMany([{ x: 1 }, { x: 2 }], configuration.writeConcernMax()); + + // Attempt to rename the first collection to the second one, this will fail + err = await collection1.rename('test_rename_collection2_with_promise').catch(e => e); + expect(err).to.be.instanceOf(Error); + expect(err.message).to.have.length.gt(0); + + // Attempt to rename the first collection to a name that does not exist + // this will be successful + collection1 = await collection1.rename('test_rename_collection3_with_promise'); + + // Ensure that the collection is pointing to the new one + expect(collection1.collectionName).to.equal('test_rename_collection3_with_promise'); + + expect(await collection1.count()).equals(2); + + /* es-lint-enable */ // END - /* eslint-enable */ } }); From 581ec25a8d9eff90198946f6d43aefc729ed2514 Mon Sep 17 00:00:00 2001 From: Warren James Date: Tue, 1 Aug 2023 15:26:36 -0400 Subject: [PATCH 14/18] fix(NODE-3568): Fix types --- src/collection.ts | 12 +-- .../community/collection/findX.test-d.ts | 83 +++++++++---------- test/types/community/transaction.test-d.ts | 8 +- test/types/schema_helpers.test-d.ts | 63 +++++++++++--- 4 files changed, 97 insertions(+), 69 deletions(-) diff --git a/src/collection.ts b/src/collection.ts index c6778cc5412..57ede865c9e 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -819,8 +819,8 @@ export class Collection { async findOneAndDelete( filter: Filter, options: FindOneAndDeleteOptions - ): Promise>; - async findOneAndDelete(filter: Filter): Promise>; + ): Promise | null>; + async findOneAndDelete(filter: Filter): Promise | null>; async findOneAndDelete( filter: Filter, options?: FindOneAndDeleteOptions @@ -856,11 +856,11 @@ export class Collection { filter: Filter, replacement: WithoutId, options: FindOneAndReplaceOptions - ): Promise>; + ): Promise | null>; async findOneAndReplace( filter: Filter, replacement: WithoutId - ): Promise>; + ): Promise | null>; async findOneAndReplace( filter: Filter, replacement: WithoutId, @@ -898,11 +898,11 @@ export class Collection { filter: Filter, update: UpdateFilter, options: FindOneAndUpdateOptions - ): Promise>; + ): Promise | null>; async findOneAndUpdate( filter: Filter, update: UpdateFilter - ): Promise>; + ): Promise | null>; async findOneAndUpdate( filter: Filter, update: UpdateFilter, diff --git a/test/types/community/collection/findX.test-d.ts b/test/types/community/collection/findX.test-d.ts index 8c546cc4e2f..6c4a6a073d2 100644 --- a/test/types/community/collection/findX.test-d.ts +++ b/test/types/community/collection/findX.test-d.ts @@ -289,42 +289,36 @@ 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.findOneAndDelete({ a: 3 })); expectType | null>( - (await coll.findOneAndReplace({ a: 3 }, { a: 5, b: 'new string' })).value + await coll.findOneAndReplace({ a: 3 }, { a: 5, b: 'new string' }) ); expectType | null>( - ( - await coll.findOneAndUpdate( - { a: 3 }, - { - $set: { - a: 5 - } + await coll.findOneAndUpdate( + { a: 3 }, + { + $set: { + a: 5 } - ) - ).value + } + ) ); // Passing empty options will return the modify result. +expectType | null>(await coll.findOneAndDelete({ a: 3 }, {})); expectType | null>( - (await coll.findOneAndDelete({ a: 3 }, {})).value + await coll.findOneAndReplace({ a: 3 }, { a: 5, b: 'new string' }, {}) ); expectType | null>( - (await coll.findOneAndReplace({ a: 3 }, { a: 5, b: 'new string' }, {})).value -); -expectType | null>( - ( - await coll.findOneAndUpdate( - { a: 3 }, - { - $set: { - a: 5 - } - }, - {} - ) - ).value + await coll.findOneAndUpdate( + { a: 3 }, + { + $set: { + a: 5 + } + }, + {} + ) ); // Including { includeResultMetadata: true } option will return the @@ -371,23 +365,20 @@ expectType | null>( // projections do not change the return type - our typing doesn't support this expectType | null>( - (await coll.findOneAndDelete({ a: 3 }, { projection: { _id: 0 } })).value -); - -// NODE-3568: Uncomment when ModifyResult is removed in 5.0 -// expectType | null> | null>((await coll.findOneAndDelete({ a: 3 }))); -// expectType | null>( -// (await coll.findOneAndReplace({ a: 3 }, { a: 5, b: 'new string' })) -// ); -// expectType | null>( -// ( -// await coll.findOneAndUpdate( -// { a: 3 }, -// { -// $set: { -// a: 5 -// } -// } -// ) -// ) -// ); + await coll.findOneAndDelete({ a: 3 }, { projection: { _id: 0 } }) +); + +expectType | null | null>(await coll.findOneAndDelete({ a: 3 })); +expectType | null>( + await coll.findOneAndReplace({ a: 3 }, { a: 5, b: 'new string' }) +); +expectType | null>( + await coll.findOneAndUpdate( + { a: 3 }, + { + $set: { + a: 5 + } + } + ) +); diff --git a/test/types/community/transaction.test-d.ts b/test/types/community/transaction.test-d.ts index d329b09ab2e..50cd9133b3a 100644 --- a/test/types/community/transaction.test-d.ts +++ b/test/types/community/transaction.test-d.ts @@ -82,11 +82,9 @@ 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) - ).value; + const res = await db + .collection('Account') + .findOneAndUpdate({ name: from }, { $inc: { balance: -amount } }, opts); const A = res; if (A?.balance && A.balance < 0) { // If A would have negative balance, fail and abort the transaction diff --git a/test/types/schema_helpers.test-d.ts b/test/types/schema_helpers.test-d.ts index aac3e0ba3ba..86714586732 100644 --- a/test/types/schema_helpers.test-d.ts +++ b/test/types/schema_helpers.test-d.ts @@ -72,22 +72,36 @@ expectAssignable(await interfaceTestCollection.fin expectAssignable((await typeTestCollection.find().toArray())[0]); expectAssignable((await interfaceTestCollection.find().toArray())[0]); expectAssignable( - (await typeTestCollection.findOneAndDelete({ a: 1 })).value + (await typeTestCollection.findOneAndDelete({ a: 1 }, { includeResultMetadata: true })).value ); expectAssignable( - (await interfaceTestCollection.findOneAndDelete({ a: 1 })).value + (await interfaceTestCollection.findOneAndDelete({ a: 1 }, { includeResultMetadata: true })).value ); expectAssignable( - (await typeTestCollection.findOneAndReplace({ a: 1 }, { a: 5 })).value + (await typeTestCollection.findOneAndReplace({ a: 1 }, { a: 5 }, { includeResultMetadata: true })) + .value ); expectAssignable( - (await interfaceTestCollection.findOneAndReplace({ a: 1 }, { a: 5 })).value + ( + await interfaceTestCollection.findOneAndReplace( + { a: 1 }, + { a: 5 }, + { includeResultMetadata: true } + ) + ).value ); expectAssignable( - (await typeTestCollection.findOneAndUpdate({ a: 1 }, { a: 5 })).value + (await typeTestCollection.findOneAndUpdate({ a: 1 }, { a: 5 }, { includeResultMetadata: true })) + .value ); expectAssignable( - (await interfaceTestCollection.findOneAndUpdate({ a: 1 }, { a: 5 })).value + ( + await interfaceTestCollection.findOneAndUpdate( + { a: 1 }, + { a: 5 }, + { includeResultMetadata: true } + ) + ).value ); // OptionalId assignability when wrapping a schema with _id: number @@ -109,22 +123,47 @@ expectAssignable( (await interfaceNumberTestCollection.find().toArray())[0] ); expectAssignable( - (await typeNumberTestCollection.findOneAndDelete({ a: 1 })).value + (await typeNumberTestCollection.findOneAndDelete({ a: 1 }, { includeResultMetadata: true })).value ); expectAssignable( - (await interfaceNumberTestCollection.findOneAndDelete({ a: 1 })).value + (await interfaceNumberTestCollection.findOneAndDelete({ a: 1 }, { includeResultMetadata: true })) + .value ); expectAssignable( - (await typeNumberTestCollection.findOneAndReplace({ a: 1 }, { a: 5 })).value + ( + await typeNumberTestCollection.findOneAndReplace( + { a: 1 }, + { a: 5 }, + { includeResultMetadata: true } + ) + ).value ); expectAssignable( - (await interfaceNumberTestCollection.findOneAndReplace({ a: 1 }, { a: 5 })).value + ( + await interfaceNumberTestCollection.findOneAndReplace( + { a: 1 }, + { a: 5 }, + { includeResultMetadata: true } + ) + ).value ); expectAssignable( - (await typeNumberTestCollection.findOneAndUpdate({ a: 1 }, { a: 5 })).value + ( + await typeNumberTestCollection.findOneAndUpdate( + { a: 1 }, + { a: 5 }, + { includeResultMetadata: true } + ) + ).value ); expectAssignable( - (await interfaceNumberTestCollection.findOneAndUpdate({ a: 1 }, { a: 5 })).value + ( + await interfaceNumberTestCollection.findOneAndUpdate( + { a: 1 }, + { a: 5 }, + { includeResultMetadata: true } + ) + ).value ); /** ---------------------------------------------------------------------- From 7f8a7a9ae78952316fb9c81c7d3ced0677813479 Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 2 Aug 2023 09:24:02 -0400 Subject: [PATCH 15/18] Apply suggestions from code review Co-authored-by: Durran Jordan --- .../community/collection/findX.test-d.ts | 4 +- test/types/schema_helpers.test-d.ts | 52 ++++--------------- 2 files changed, 12 insertions(+), 44 deletions(-) diff --git a/test/types/community/collection/findX.test-d.ts b/test/types/community/collection/findX.test-d.ts index 6c4a6a073d2..1a5eada8802 100644 --- a/test/types/community/collection/findX.test-d.ts +++ b/test/types/community/collection/findX.test-d.ts @@ -288,7 +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. +// Passing no options will return the deleted document. expectType | null>(await coll.findOneAndDelete({ a: 3 })); expectType | null>( await coll.findOneAndReplace({ a: 3 }, { a: 5, b: 'new string' }) @@ -304,7 +304,7 @@ expectType | null>( ) ); -// Passing empty options will return the modify result. +// Passing empty options will return the deleted document. expectType | null>(await coll.findOneAndDelete({ a: 3 }, {})); expectType | null>( await coll.findOneAndReplace({ a: 3 }, { a: 5, b: 'new string' }, {}) diff --git a/test/types/schema_helpers.test-d.ts b/test/types/schema_helpers.test-d.ts index 86714586732..1cbd87a5eec 100644 --- a/test/types/schema_helpers.test-d.ts +++ b/test/types/schema_helpers.test-d.ts @@ -72,14 +72,13 @@ expectAssignable(await interfaceTestCollection.fin expectAssignable((await typeTestCollection.find().toArray())[0]); expectAssignable((await interfaceTestCollection.find().toArray())[0]); expectAssignable( - (await typeTestCollection.findOneAndDelete({ a: 1 }, { includeResultMetadata: true })).value + await typeTestCollection.findOneAndDelete({ a: 1 }) ); expectAssignable( - (await interfaceTestCollection.findOneAndDelete({ a: 1 }, { includeResultMetadata: true })).value + await interfaceTestCollection.findOneAndDelete({ a: 1 }) ); expectAssignable( - (await typeTestCollection.findOneAndReplace({ a: 1 }, { a: 5 }, { includeResultMetadata: true })) - .value + await typeTestCollection.findOneAndReplace({ a: 1 }, { a: 5 }) ); expectAssignable( ( @@ -95,13 +94,7 @@ expectAssignable( .value ); expectAssignable( - ( - await interfaceTestCollection.findOneAndUpdate( - { a: 1 }, - { a: 5 }, - { includeResultMetadata: true } - ) - ).value + await interfaceTestCollection.findOneAndUpdate({ a: 1 },{ a: 5 }) ); // OptionalId assignability when wrapping a schema with _id: number @@ -123,47 +116,22 @@ expectAssignable( (await interfaceNumberTestCollection.find().toArray())[0] ); expectAssignable( - (await typeNumberTestCollection.findOneAndDelete({ a: 1 }, { includeResultMetadata: true })).value + await typeNumberTestCollection.findOneAndDelete({ a: 1 }) ); expectAssignable( - (await interfaceNumberTestCollection.findOneAndDelete({ a: 1 }, { includeResultMetadata: true })) - .value + await interfaceNumberTestCollection.findOneAndDelete({ a: 1 }) ); expectAssignable( - ( - await typeNumberTestCollection.findOneAndReplace( - { a: 1 }, - { a: 5 }, - { includeResultMetadata: true } - ) - ).value + await typeNumberTestCollection.findOneAndReplace({ a: 1 }, { a: 5 }) ); expectAssignable( - ( - await interfaceNumberTestCollection.findOneAndReplace( - { a: 1 }, - { a: 5 }, - { includeResultMetadata: true } - ) - ).value + await interfaceNumberTestCollection.findOneAndReplace({ a: 1 }, { a: 5 }) ); expectAssignable( - ( - await typeNumberTestCollection.findOneAndUpdate( - { a: 1 }, - { a: 5 }, - { includeResultMetadata: true } - ) - ).value + await typeNumberTestCollection.findOneAndUpdate({ a: 1 }, { a: 5 }) ); expectAssignable( - ( - await interfaceNumberTestCollection.findOneAndUpdate( - { a: 1 }, - { a: 5 }, - { includeResultMetadata: true } - ) - ).value + await interfaceNumberTestCollection.findOneAndUpdate({ a: 1 }, { a: 5 }) ); /** ---------------------------------------------------------------------- From 4b260cc29ea221b12560cb862e34c7aa6828a168 Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 2 Aug 2023 09:28:02 -0400 Subject: [PATCH 16/18] style(NODE-3568): eslint --- test/types/schema_helpers.test-d.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/types/schema_helpers.test-d.ts b/test/types/schema_helpers.test-d.ts index 1cbd87a5eec..b2ef9d1e081 100644 --- a/test/types/schema_helpers.test-d.ts +++ b/test/types/schema_helpers.test-d.ts @@ -71,9 +71,7 @@ expectAssignable(await typeTestCollection.findOne()); expectAssignable(await interfaceTestCollection.findOne()); expectAssignable((await typeTestCollection.find().toArray())[0]); expectAssignable((await interfaceTestCollection.find().toArray())[0]); -expectAssignable( - await typeTestCollection.findOneAndDelete({ a: 1 }) -); +expectAssignable(await typeTestCollection.findOneAndDelete({ a: 1 })); expectAssignable( await interfaceTestCollection.findOneAndDelete({ a: 1 }) ); @@ -94,7 +92,7 @@ expectAssignable( .value ); expectAssignable( - await interfaceTestCollection.findOneAndUpdate({ a: 1 },{ a: 5 }) + await interfaceTestCollection.findOneAndUpdate({ a: 1 }, { a: 5 }) ); // OptionalId assignability when wrapping a schema with _id: number From 9b3d7df8cf75d4bf755bcf5d62ed05ea9c00eb21 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 3 Aug 2023 10:11:09 -0400 Subject: [PATCH 17/18] Apply suggestions from code review Co-authored-by: Durran Jordan --- test/types/schema_helpers.test-d.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/test/types/schema_helpers.test-d.ts b/test/types/schema_helpers.test-d.ts index b2ef9d1e081..ec82a4ba18f 100644 --- a/test/types/schema_helpers.test-d.ts +++ b/test/types/schema_helpers.test-d.ts @@ -79,17 +79,10 @@ expectAssignable( await typeTestCollection.findOneAndReplace({ a: 1 }, { a: 5 }) ); expectAssignable( - ( - await interfaceTestCollection.findOneAndReplace( - { a: 1 }, - { a: 5 }, - { includeResultMetadata: true } - ) - ).value + await interfaceTestCollection.findOneAndReplace({ a: 1 }, { a: 5 }) ); expectAssignable( - (await typeTestCollection.findOneAndUpdate({ a: 1 }, { a: 5 }, { includeResultMetadata: true })) - .value + await typeTestCollection.findOneAndUpdate({ a: 1 }, { a: 5 }) ); expectAssignable( await interfaceTestCollection.findOneAndUpdate({ a: 1 }, { a: 5 }) From 2b79dff9f6499afab99badcc48b41bb81b0e04dc Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 3 Aug 2023 10:11:57 -0400 Subject: [PATCH 18/18] style(NODE-3568): eslint --- test/types/schema_helpers.test-d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/types/schema_helpers.test-d.ts b/test/types/schema_helpers.test-d.ts index ec82a4ba18f..6c09ee07900 100644 --- a/test/types/schema_helpers.test-d.ts +++ b/test/types/schema_helpers.test-d.ts @@ -79,7 +79,7 @@ expectAssignable( await typeTestCollection.findOneAndReplace({ a: 1 }, { a: 5 }) ); expectAssignable( - await interfaceTestCollection.findOneAndReplace({ a: 1 }, { a: 5 }) + await interfaceTestCollection.findOneAndReplace({ a: 1 }, { a: 5 }) ); expectAssignable( await typeTestCollection.findOneAndUpdate({ a: 1 }, { a: 5 })