diff --git a/docs/reference/content/reference/ecmascriptnext/crud.md b/docs/reference/content/reference/ecmascriptnext/crud.md index 624e4ebbe81..479b42b648f 100644 --- a/docs/reference/content/reference/ecmascriptnext/crud.md +++ b/docs/reference/content/reference/ecmascriptnext/crud.md @@ -213,7 +213,7 @@ const dbName = 'myproject'; // Modify and return the modified document r = await col.findOneAndUpdate({a:1}, {$set: {b: 1}}, { - returnOriginal: false, + returnDocument: 'after', sort: [['a',1]], upsert: true }); diff --git a/docs/reference/content/tutorials/crud.md b/docs/reference/content/tutorials/crud.md index 13a87e6deeb..941f29bc880 100644 --- a/docs/reference/content/tutorials/crud.md +++ b/docs/reference/content/tutorials/crud.md @@ -354,7 +354,7 @@ methods, the operation takes a write lock for the duration of the operation in o // Modify and return the modified document col.findOneAndUpdate({a:1}, {$set: {b: 1}}, { - returnOriginal: false + returnDocument: 'after' , sort: [[a,1]] , upsert: true }, function(err, r) { @@ -382,7 +382,7 @@ methods, the operation takes a write lock for the duration of the operation in o // Modify and return the modified document r = await col.findOneAndUpdate({a:1}, {$set: {b: 1}}, { - returnOriginal: false + returnDocument: 'after' , sort: [[a,1]] , upsert: true }); @@ -410,7 +410,7 @@ The ``findOneAndUpdate`` method also accepts a third argument which can be an op | `upsert` | (Boolean, default:false) | Perform an upsert operation. | | `sort` | (Object, default:null) | Sort for find operation. | | `projection` | (Object, default:null) | Projection for returned result | -| `returnOriginal` | (Boolean, default:true) | Set to false if you want to return the modified object rather than the original. Ignored for remove. | +| `returnDocument` | (String, 'before' \|\| 'after', default:'before') | Set to 'after' if you want to return the modified document rather than the original. Ignored for remove. | The ``findOneAndDelete`` function is designed to help remove a document. diff --git a/lib/collection.js b/lib/collection.js index 87005c0e539..09b8304b684 100644 --- a/lib/collection.js +++ b/lib/collection.js @@ -1655,7 +1655,7 @@ Collection.prototype.stats = function(options, callback) { /** * @typedef {Object} Collection~findAndModifyWriteOpResult - * @property {object} value Document returned from the `findAndModify` command. If no documents were found, `value` will be `null` by default (`returnOriginal: true`), even if a document was upserted; if `returnOriginal` was false, the upserted document will be returned in that case. + * @property {object} value Document returned from the `findAndModify` command. If no documents were found, `value` will be `null` by default even if a document was upserted unless `returnDocument` is specified as `'after'`, in which case the upserted document will be returned. * @property {object} lastErrorObject The raw lastErrorObject returned from the command. See {@link https://docs.mongodb.com/manual/reference/command/findAndModify/index.html#lasterrorobject|findAndModify command documentation}. * @property {Number} ok Is 1 if the command executed correctly. */ @@ -1716,7 +1716,8 @@ Collection.prototype.findOneAndDelete = function(filter, options, callback) { * @param {object} [options.projection] Limits the fields to return for all matching documents. * @param {object} [options.sort] Determines which document the operation modifies if the query selects multiple documents. * @param {boolean} [options.upsert=false] Upsert the document if it does not exist. - * @param {boolean} [options.returnOriginal=true] When false, returns the updated document rather than the original. The default is true. + * @param {'before'|'after'} [options.returnDocument='before'] When set to `'after'`, returns the updated document rather than the original. The default is `'before'`. + * @param {boolean} [options.returnOriginal=true] **Deprecated** Use `options.returnDocument` instead. * @param {boolean} [options.checkKeys=false] If true, will throw if bson documents start with `$` or include a `.` in any key value * @param {boolean} [options.serializeFunctions=false] Serialize functions on any object. * @param {boolean} [options.ignoreUndefined=false] Specify if the BSON serializer should ignore undefined fields. @@ -1725,22 +1726,29 @@ Collection.prototype.findOneAndDelete = function(filter, options, callback) { * @param {Collection~findAndModifyCallback} [callback] The collection result callback * @return {Promise} returns Promise if no callback passed */ -Collection.prototype.findOneAndReplace = function(filter, replacement, options, callback) { - if (typeof options === 'function') (callback = options), (options = {}); - options = options || {}; +Collection.prototype.findOneAndReplace = deprecateOptions( + { + name: 'collection.findOneAndReplace', + deprecatedOptions: ['returnOriginal'], + optionsIndex: 2 + }, + function(filter, replacement, options, callback) { + if (typeof options === 'function') (callback = options), (options = {}); + options = options || {}; - // Add ignoreUndefined - if (this.s.options.ignoreUndefined) { - options = Object.assign({}, options); - options.ignoreUndefined = this.s.options.ignoreUndefined; - } + // Add ignoreUndefined + if (this.s.options.ignoreUndefined) { + options = Object.assign({}, options); + options.ignoreUndefined = this.s.options.ignoreUndefined; + } - return executeOperation( - this.s.topology, - new FindOneAndReplaceOperation(this, filter, replacement, options), - callback - ); -}; + return executeOperation( + this.s.topology, + new FindOneAndReplaceOperation(this, filter, replacement, options), + callback + ); + } +); /** * Find a document and update it in one atomic operation. Requires a write lock for the duration of the operation. @@ -1757,7 +1765,8 @@ Collection.prototype.findOneAndReplace = function(filter, replacement, options, * @param {object} [options.projection] Limits the fields to return for all matching documents. * @param {object} [options.sort] Determines which document the operation modifies if the query selects multiple documents. * @param {boolean} [options.upsert=false] Upsert the document if it does not exist. - * @param {boolean} [options.returnOriginal=true] When false, returns the updated document rather than the original. The default is true. + * @param {'before'|'after'} [options.returnDocument='before'] When set to `'after'`, returns the updated document rather than the original. The default is `'before'`. + * @param {boolean} [options.returnOriginal=true] **Deprecated** Use `options.returnDocument` instead. * @param {boolean} [options.checkKeys=false] If true, will throw if bson documents start with `$` or include a `.` in any key value * @param {boolean} [options.serializeFunctions=false] Serialize functions on any object. * @param {boolean} [options.ignoreUndefined=false] Specify if the BSON serializer should ignore undefined fields. @@ -1766,22 +1775,29 @@ Collection.prototype.findOneAndReplace = function(filter, replacement, options, * @param {Collection~findAndModifyCallback} [callback] The collection result callback * @return {Promise} returns Promise if no callback passed */ -Collection.prototype.findOneAndUpdate = function(filter, update, options, callback) { - if (typeof options === 'function') (callback = options), (options = {}); - options = options || {}; +Collection.prototype.findOneAndUpdate = deprecateOptions( + { + name: 'collection.findOneAndUpdate', + deprecatedOptions: ['returnOriginal'], + optionsIndex: 2 + }, + function(filter, update, options, callback) { + if (typeof options === 'function') (callback = options), (options = {}); + options = options || {}; - // Add ignoreUndefined - if (this.s.options.ignoreUndefined) { - options = Object.assign({}, options); - options.ignoreUndefined = this.s.options.ignoreUndefined; - } + // Add ignoreUndefined + if (this.s.options.ignoreUndefined) { + options = Object.assign({}, options); + options.ignoreUndefined = this.s.options.ignoreUndefined; + } - return executeOperation( - this.s.topology, - new FindOneAndUpdateOperation(this, filter, update, options), - callback - ); -}; + return executeOperation( + this.s.topology, + new FindOneAndUpdateOperation(this, filter, update, options), + callback + ); + } +); /** * Find and update a document. diff --git a/lib/operations/find_one_and_replace.js b/lib/operations/find_one_and_replace.js index f068cdfbcbf..3720baea4ee 100644 --- a/lib/operations/find_one_and_replace.js +++ b/lib/operations/find_one_and_replace.js @@ -1,16 +1,22 @@ 'use strict'; +const MongoError = require('../core').MongoError; const FindAndModifyOperation = require('./find_and_modify'); const hasAtomicOperators = require('../utils').hasAtomicOperators; class FindOneAndReplaceOperation extends FindAndModifyOperation { constructor(collection, filter, replacement, options) { + if ('returnDocument' in options && 'returnOriginal' in options) { + throw new MongoError( + 'findOneAndReplace option returnOriginal is deprecated in favor of returnDocument and cannot be combined' + ); + } // Final options const finalOptions = Object.assign({}, options); finalOptions.fields = options.projection; finalOptions.update = true; - finalOptions.new = options.returnOriginal !== void 0 ? !options.returnOriginal : false; - finalOptions.upsert = options.upsert !== void 0 ? !!options.upsert : false; + finalOptions.new = options.returnDocument === 'after' || options.returnOriginal === false; + finalOptions.upsert = options.upsert === true; if (filter == null || typeof filter !== 'object') { throw new TypeError('Filter parameter must be an object'); diff --git a/lib/operations/find_one_and_update.js b/lib/operations/find_one_and_update.js index 5ec8be5751c..9408aca7bfe 100644 --- a/lib/operations/find_one_and_update.js +++ b/lib/operations/find_one_and_update.js @@ -1,17 +1,22 @@ 'use strict'; +const MongoError = require('../core').MongoError; const FindAndModifyOperation = require('./find_and_modify'); const hasAtomicOperators = require('../utils').hasAtomicOperators; class FindOneAndUpdateOperation extends FindAndModifyOperation { constructor(collection, filter, update, options) { + if ('returnDocument' in options && 'returnOriginal' in options) { + throw new MongoError( + 'findOneAndUpdate option returnOriginal is deprecated in favor of returnDocument and cannot be combined' + ); + } // Final options const finalOptions = Object.assign({}, options); finalOptions.fields = options.projection; finalOptions.update = true; - finalOptions.new = - typeof options.returnOriginal === 'boolean' ? !options.returnOriginal : false; - finalOptions.upsert = typeof options.upsert === 'boolean' ? options.upsert : false; + finalOptions.new = options.returnDocument === 'after' || options.returnOriginal === false; + finalOptions.upsert = options.upsert === true; if (filter == null || typeof filter !== 'object') { throw new TypeError('Filter parameter must be an object'); diff --git a/test/functional/crud_api.test.js b/test/functional/crud_api.test.js index 5f2051af93e..3269d6d4481 100644 --- a/test/functional/crud_api.test.js +++ b/test/functional/crud_api.test.js @@ -766,7 +766,7 @@ describe('CRUD API', function() { { projection: { b: 1, c: 1 }, sort: { a: 1 }, - returnOriginal: false, + returnOriginal: false, // keeping the deprecated option for compatibility testing since returnDocument is covered in spec tests upsert: true }, function(err, r) { @@ -795,7 +795,7 @@ describe('CRUD API', function() { { projection: { b: 1, d: 1 }, sort: { a: 1 }, - returnOriginal: false, + returnOriginal: false, // keeping the deprecated option for compatibility testing since returnDocument is covered in spec tests upsert: true }, function(err, r) { diff --git a/test/functional/crud_spec.test.js b/test/functional/crud_spec.test.js index e531a2ba357..095e5474c9d 100644 --- a/test/functional/crud_spec.test.js +++ b/test/functional/crud_spec.test.js @@ -359,13 +359,12 @@ describe('CRUD spec', function() { const second = args.update || args.replacement; const options = Object.assign({}, args); if (options.returnDocument) { - options.returnOriginal = options.returnDocument === 'After' ? false : true; + options.returnDocument = options.returnDocument.toLowerCase(); } delete options.filter; delete options.update; delete options.replacement; - delete options.returnDocument; const opName = scenarioTest.operation.name; const findPromise = diff --git a/test/functional/operation_example.test.js b/test/functional/operation_example.test.js index 001e6d8a09e..4366c600159 100644 --- a/test/functional/operation_example.test.js +++ b/test/functional/operation_example.test.js @@ -9269,7 +9269,7 @@ describe('Operation Examples', function() { { projection: { b: 1, c: 1 }, sort: { a: 1 }, - returnOriginal: false, + returnDocument: 'after', upsert: true }, function(err, r) { @@ -9327,7 +9327,7 @@ describe('Operation Examples', function() { { projection: { b: 1, d: 1 }, sort: { a: 1 }, - returnOriginal: false, + returnDocument: 'after', upsert: true }, function(err, r) { diff --git a/test/functional/operation_generators_example.test.js b/test/functional/operation_generators_example.test.js index f1a9cd596ad..af9e0a09015 100644 --- a/test/functional/operation_generators_example.test.js +++ b/test/functional/operation_generators_example.test.js @@ -6385,7 +6385,7 @@ describe('Operation (Generators)', function() { { projection: { b: 1, c: 1 }, sort: { a: 1 }, - returnOriginal: false, + returnDocument: 'after', upsert: true } ); @@ -6442,7 +6442,7 @@ describe('Operation (Generators)', function() { { projection: { b: 1, d: 1 }, sort: { a: 1 }, - returnOriginal: false, + returnDocument: 'after', upsert: true } ); diff --git a/test/functional/operation_promises_example.test.js b/test/functional/operation_promises_example.test.js index 2f3bbad32e6..9264e438960 100644 --- a/test/functional/operation_promises_example.test.js +++ b/test/functional/operation_promises_example.test.js @@ -6953,7 +6953,7 @@ describe('Operation (Promises)', function() { { projection: { b: 1, c: 1 }, sort: { a: 1 }, - returnOriginal: false, + returnDocument: 'after', upsert: true } ) @@ -7011,7 +7011,7 @@ describe('Operation (Promises)', function() { { projection: { b: 1, d: 1 }, sort: { a: 1 }, - returnOriginal: false, + returnDocument: 'after', upsert: true } ); diff --git a/test/functional/retryable_writes.test.js b/test/functional/retryable_writes.test.js index a3dc0bfba29..2fa1920c060 100644 --- a/test/functional/retryable_writes.test.js +++ b/test/functional/retryable_writes.test.js @@ -131,7 +131,7 @@ function generateArguments(test) { options.upsert = test.operation.arguments[arg]; } else if (arg === 'returnDocument') { const returnDocument = test.operation.arguments[arg]; - options.returnOriginal = returnDocument === 'Before'; + options.returnDocument = returnDocument.toLowerCase(); } else { args.push(test.operation.arguments[arg]); } diff --git a/test/functional/spec-runner/index.js b/test/functional/spec-runner/index.js index c2cdad9f37d..179f30c6c1d 100644 --- a/test/functional/spec-runner/index.js +++ b/test/functional/spec-runner/index.js @@ -661,7 +661,7 @@ function testOperation(operation, obj, context, options) { } if (key === 'returnDocument') { - opOptions.returnOriginal = operation.arguments[key] === 'Before' ? true : false; + opOptions.returnDocument = operation.arguments[key].toLowerCase(); return; } diff --git a/test/unit/collection.test.js b/test/unit/collection.test.js index 5d6c0100f70..139ac6edbf4 100644 --- a/test/unit/collection.test.js +++ b/test/unit/collection.test.js @@ -16,17 +16,48 @@ class MockTopology extends EventEmitter { } describe('Collection', function() { - /** - * @ignore - */ - it('should not allow atomic operators for findOneAndReplace', { - metadata: { requires: { topology: 'single' } }, - test: function() { - const db = new Db('fakeDb', new MockTopology()); - const collection = db.collection('test'); - expect(() => { - collection.findOneAndReplace({ a: 1 }, { $set: { a: 14 } }); - }).to.throw(/must not contain atomic operators/); - } + describe('findOneAndReplace()', function() { + it('should throw on atomic operators in replacement document', { + metadata: { requires: { topology: 'single' } }, + test: function() { + const db = new Db('fakeDb', new MockTopology()); + const collection = db.collection('test'); + expect(() => { + collection.findOneAndReplace({ a: 1 }, { $set: { a: 14 } }); + }).to.throw(/must not contain atomic operators/); + } + }); + + it('should throw if returnOriginal is specified with returnDocument as an option', { + metadata: { requires: { topology: 'single' } }, + test: function() { + const db = new Db('fakeDb', new MockTopology()); + const collection = db.collection('test'); + expect(() => { + collection.findOneAndReplace( + { a: 1 }, + { b: 2 }, + { returnOriginal: false, returnDocument: 'after' } + ); + }).to.throw(/returnOriginal is deprecated in favor of returnDocument/); + } + }); + }); + + describe('findOneAndUpdate()', function() { + it('should throw if returnOriginal is specified with returnDocument as an option', { + metadata: { requires: { topology: 'single' } }, + test: function() { + const db = new Db('fakeDb', new MockTopology()); + const collection = db.collection('test'); + expect(() => { + collection.findOneAndUpdate( + { a: 1 }, + { $set: { a: 14 } }, + { returnOriginal: true, returnDocument: 'before' } + ); + }).to.throw(/returnOriginal is deprecated in favor of returnDocument/); + } + }); }); });