Skip to content

Commit 8a88a1f

Browse files
dariakpdariakp
committed
refactor(NODE-1812): Deprecate returnOriginal in favor of returnDocument option in findOneAndReplace and findOneAndUpdate methods
1 parent 945e915 commit 8a88a1f

File tree

14 files changed

+122
-65
lines changed

14 files changed

+122
-65
lines changed

‎docs/reference/content/reference/ecmascriptnext/crud.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ const dbName = 'myproject';
213213

214214
// Modify and return the modified document
215215
r = await col.findOneAndUpdate({a:1}, {$set: {b: 1}}, {
216-
returnOriginal: false,
216+
returnDocument: 'after',
217217
sort: [['a',1]],
218218
upsert: true
219219
});

‎docs/reference/content/tutorials/crud.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ methods, the operation takes a write lock for the duration of the operation in o
354354

355355
// Modify and return the modified document
356356
col.findOneAndUpdate({a:1}, {$set: {b: 1}}, {
357-
returnOriginal: false
357+
returnDocument: 'after'
358358
, sort: [[a,1]]
359359
, upsert: true
360360
}, function(err, r) {
@@ -382,7 +382,7 @@ methods, the operation takes a write lock for the duration of the operation in o
382382

383383
// Modify and return the modified document
384384
r = await col.findOneAndUpdate({a:1}, {$set: {b: 1}}, {
385-
returnOriginal: false
385+
returnDocument: 'after'
386386
, sort: [[a,1]]
387387
, upsert: true
388388
});
@@ -410,7 +410,7 @@ The ``findOneAndUpdate`` method also accepts a third argument which can be an op
410410
| `upsert` | (Boolean, default:false) | Perform an upsert operation. |
411411
| `sort` | (Object, default:null) | Sort for find operation. |
412412
| `projection` | (Object, default:null) | Projection for returned result |
413-
| `returnOriginal` | (Boolean, default:true) | Set to false if you want to return the modified object rather than the original. Ignored for remove. |
413+
| `returnDocument` | (String, 'before' \|\| 'after', default:'before') | Set to 'after' if you want to return the modified document rather than the original. Ignored for remove. |
414414

415415
The ``findOneAndDelete`` function is designed to help remove a document.
416416

‎lib/collection.js

Lines changed: 47 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1655,7 +1655,7 @@ Collection.prototype.stats = function(options, callback) {
16551655

16561656
/**
16571657
* @typedef {Object} Collection~findAndModifyWriteOpResult
1658-
* @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.
1658+
* @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.
16591659
* @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}.
16601660
* @property {Number} ok Is 1 if the command executed correctly.
16611661
*/
@@ -1716,7 +1716,8 @@ Collection.prototype.findOneAndDelete = function(filter, options, callback) {
17161716
* @param {object} [options.projection] Limits the fields to return for all matching documents.
17171717
* @param {object} [options.sort] Determines which document the operation modifies if the query selects multiple documents.
17181718
* @param {boolean} [options.upsert=false] Upsert the document if it does not exist.
1719-
* @param {boolean} [options.returnOriginal=true] When false, returns the updated document rather than the original. The default is true.
1719+
* @param {'before'|'after'} [options.returnDocument='before'] When set to `'after'`, returns the updated document rather than the original. The default is `'before'`.
1720+
* @param {boolean} [options.returnOriginal=true] **Deprecated** Use `options.returnDocument` instead.
17201721
* @param {boolean} [options.checkKeys=false] If true, will throw if bson documents start with `$` or include a `.` in any key value
17211722
* @param {boolean} [options.serializeFunctions=false] Serialize functions on any object.
17221723
* @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) {
17251726
* @param {Collection~findAndModifyCallback} [callback] The collection result callback
17261727
* @return {Promise<Collection~findAndModifyWriteOpResultObject>} returns Promise if no callback passed
17271728
*/
1728-
Collection.prototype.findOneAndReplace = function(filter, replacement, options, callback) {
1729-
if (typeof options === 'function') (callback = options), (options = {});
1730-
options = options || {};
1729+
Collection.prototype.findOneAndReplace = deprecateOptions(
1730+
{
1731+
name: 'collection.findOneAndReplace',
1732+
deprecatedOptions: ['returnOriginal'],
1733+
optionsIndex: 2
1734+
},
1735+
function(filter, replacement, options, callback) {
1736+
if (typeof options === 'function') (callback = options), (options = {});
1737+
options = options || {};
17311738

1732-
// Add ignoreUndefined
1733-
if (this.s.options.ignoreUndefined) {
1734-
options = Object.assign({}, options);
1735-
options.ignoreUndefined = this.s.options.ignoreUndefined;
1736-
}
1739+
// Add ignoreUndefined
1740+
if (this.s.options.ignoreUndefined) {
1741+
options = Object.assign({}, options);
1742+
options.ignoreUndefined = this.s.options.ignoreUndefined;
1743+
}
17371744

1738-
return executeOperation(
1739-
this.s.topology,
1740-
new FindOneAndReplaceOperation(this, filter, replacement, options),
1741-
callback
1742-
);
1743-
};
1745+
return executeOperation(
1746+
this.s.topology,
1747+
new FindOneAndReplaceOperation(this, filter, replacement, options),
1748+
callback
1749+
);
1750+
}
1751+
);
17441752

17451753
/**
17461754
* 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,
17571765
* @param {object} [options.projection] Limits the fields to return for all matching documents.
17581766
* @param {object} [options.sort] Determines which document the operation modifies if the query selects multiple documents.
17591767
* @param {boolean} [options.upsert=false] Upsert the document if it does not exist.
1760-
* @param {boolean} [options.returnOriginal=true] When false, returns the updated document rather than the original. The default is true.
1768+
* @param {'before'|'after'} [options.returnDocument='before'] When set to `'after'`, returns the updated document rather than the original. The default is `'before'`.
1769+
* @param {boolean} [options.returnOriginal=true] **Deprecated** Use `options.returnDocument` instead.
17611770
* @param {boolean} [options.checkKeys=false] If true, will throw if bson documents start with `$` or include a `.` in any key value
17621771
* @param {boolean} [options.serializeFunctions=false] Serialize functions on any object.
17631772
* @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,
17661775
* @param {Collection~findAndModifyCallback} [callback] The collection result callback
17671776
* @return {Promise<Collection~findAndModifyWriteOpResultObject>} returns Promise if no callback passed
17681777
*/
1769-
Collection.prototype.findOneAndUpdate = function(filter, update, options, callback) {
1770-
if (typeof options === 'function') (callback = options), (options = {});
1771-
options = options || {};
1778+
Collection.prototype.findOneAndUpdate = deprecateOptions(
1779+
{
1780+
name: 'collection.findOneAndUpdate',
1781+
deprecatedOptions: ['returnOriginal'],
1782+
optionsIndex: 2
1783+
},
1784+
function(filter, update, options, callback) {
1785+
if (typeof options === 'function') (callback = options), (options = {});
1786+
options = options || {};
17721787

1773-
// Add ignoreUndefined
1774-
if (this.s.options.ignoreUndefined) {
1775-
options = Object.assign({}, options);
1776-
options.ignoreUndefined = this.s.options.ignoreUndefined;
1777-
}
1788+
// Add ignoreUndefined
1789+
if (this.s.options.ignoreUndefined) {
1790+
options = Object.assign({}, options);
1791+
options.ignoreUndefined = this.s.options.ignoreUndefined;
1792+
}
17781793

1779-
return executeOperation(
1780-
this.s.topology,
1781-
new FindOneAndUpdateOperation(this, filter, update, options),
1782-
callback
1783-
);
1784-
};
1794+
return executeOperation(
1795+
this.s.topology,
1796+
new FindOneAndUpdateOperation(this, filter, update, options),
1797+
callback
1798+
);
1799+
}
1800+
);
17851801

17861802
/**
17871803
* Find and update a document.

‎lib/operations/find_one_and_replace.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,22 @@
11
'use strict';
22

3+
const MongoError = require('../core').MongoError;
34
const FindAndModifyOperation = require('./find_and_modify');
45
const hasAtomicOperators = require('../utils').hasAtomicOperators;
56

67
class FindOneAndReplaceOperation extends FindAndModifyOperation {
78
constructor(collection, filter, replacement, options) {
9+
if ('returnDocument' in options && 'returnOriginal' in options) {
10+
throw new MongoError(
11+
'findOneAndReplace option returnOriginal is deprecated in favor of returnDocument and cannot be combined'
12+
);
13+
}
814
// Final options
915
const finalOptions = Object.assign({}, options);
1016
finalOptions.fields = options.projection;
1117
finalOptions.update = true;
12-
finalOptions.new = options.returnOriginal !== void 0 ? !options.returnOriginal : false;
13-
finalOptions.upsert = options.upsert !== void 0 ? !!options.upsert : false;
18+
finalOptions.new = options.returnDocument === 'after' || options.returnOriginal === false;
19+
finalOptions.upsert = options.upsert === true;
1420

1521
if (filter == null || typeof filter !== 'object') {
1622
throw new TypeError('Filter parameter must be an object');

‎lib/operations/find_one_and_update.js

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,22 @@
11
'use strict';
22

3+
const MongoError = require('../core').MongoError;
34
const FindAndModifyOperation = require('./find_and_modify');
45
const hasAtomicOperators = require('../utils').hasAtomicOperators;
56

67
class FindOneAndUpdateOperation extends FindAndModifyOperation {
78
constructor(collection, filter, update, options) {
9+
if ('returnDocument' in options && 'returnOriginal' in options) {
10+
throw new MongoError(
11+
'findOneAndUpdate option returnOriginal is deprecated in favor of returnDocument and cannot be combined'
12+
);
13+
}
814
// Final options
915
const finalOptions = Object.assign({}, options);
1016
finalOptions.fields = options.projection;
1117
finalOptions.update = true;
12-
finalOptions.new =
13-
typeof options.returnOriginal === 'boolean' ? !options.returnOriginal : false;
14-
finalOptions.upsert = typeof options.upsert === 'boolean' ? options.upsert : false;
18+
finalOptions.new = options.returnDocument === 'after' || options.returnOriginal === false;
19+
finalOptions.upsert = options.upsert === true;
1520

1621
if (filter == null || typeof filter !== 'object') {
1722
throw new TypeError('Filter parameter must be an object');

‎test/functional/crud_api.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -766,7 +766,7 @@ describe('CRUD API', function() {
766766
{
767767
projection: { b: 1, c: 1 },
768768
sort: { a: 1 },
769-
returnOriginal: false,
769+
returnOriginal: false, // keeping the deprecated option for compatibility testing since returnDocument is covered in spec tests
770770
upsert: true
771771
},
772772
function(err, r) {
@@ -795,7 +795,7 @@ describe('CRUD API', function() {
795795
{
796796
projection: { b: 1, d: 1 },
797797
sort: { a: 1 },
798-
returnOriginal: false,
798+
returnOriginal: false, // keeping the deprecated option for compatibility testing since returnDocument is covered in spec tests
799799
upsert: true
800800
},
801801
function(err, r) {

‎test/functional/crud_spec.test.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -359,13 +359,12 @@ describe('CRUD spec', function() {
359359
const second = args.update || args.replacement;
360360
const options = Object.assign({}, args);
361361
if (options.returnDocument) {
362-
options.returnOriginal = options.returnDocument === 'After' ? false : true;
362+
options.returnDocument = options.returnDocument.toLowerCase();
363363
}
364364

365365
delete options.filter;
366366
delete options.update;
367367
delete options.replacement;
368-
delete options.returnDocument;
369368

370369
const opName = scenarioTest.operation.name;
371370
const findPromise =

‎test/functional/operation_example.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9269,7 +9269,7 @@ describe('Operation Examples', function() {
92699269
{
92709270
projection: { b: 1, c: 1 },
92719271
sort: { a: 1 },
9272-
returnOriginal: false,
9272+
returnDocument: 'after',
92739273
upsert: true
92749274
},
92759275
function(err, r) {
@@ -9327,7 +9327,7 @@ describe('Operation Examples', function() {
93279327
{
93289328
projection: { b: 1, d: 1 },
93299329
sort: { a: 1 },
9330-
returnOriginal: false,
9330+
returnDocument: 'after',
93319331
upsert: true
93329332
},
93339333
function(err, r) {

‎test/functional/operation_generators_example.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6385,7 +6385,7 @@ describe('Operation (Generators)', function() {
63856385
{
63866386
projection: { b: 1, c: 1 },
63876387
sort: { a: 1 },
6388-
returnOriginal: false,
6388+
returnDocument: 'after',
63896389
upsert: true
63906390
}
63916391
);
@@ -6442,7 +6442,7 @@ describe('Operation (Generators)', function() {
64426442
{
64436443
projection: { b: 1, d: 1 },
64446444
sort: { a: 1 },
6445-
returnOriginal: false,
6445+
returnDocument: 'after',
64466446
upsert: true
64476447
}
64486448
);

‎test/functional/operation_promises_example.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6953,7 +6953,7 @@ describe('Operation (Promises)', function() {
69536953
{
69546954
projection: { b: 1, c: 1 },
69556955
sort: { a: 1 },
6956-
returnOriginal: false,
6956+
returnDocument: 'after',
69576957
upsert: true
69586958
}
69596959
)
@@ -7011,7 +7011,7 @@ describe('Operation (Promises)', function() {
70117011
{
70127012
projection: { b: 1, d: 1 },
70137013
sort: { a: 1 },
7014-
returnOriginal: false,
7014+
returnDocument: 'after',
70157015
upsert: true
70167016
}
70177017
);

‎test/functional/retryable_writes.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ function generateArguments(test) {
131131
options.upsert = test.operation.arguments[arg];
132132
} else if (arg === 'returnDocument') {
133133
const returnDocument = test.operation.arguments[arg];
134-
options.returnOriginal = returnDocument === 'Before';
134+
options.returnDocument = returnDocument.toLowerCase();
135135
} else {
136136
args.push(test.operation.arguments[arg]);
137137
}

‎test/functional/spec-runner/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,7 @@ function testOperation(operation, obj, context, options) {
661661
}
662662

663663
if (key === 'returnDocument') {
664-
opOptions.returnOriginal = operation.arguments[key] === 'Before' ? true : false;
664+
opOptions.returnDocument = operation.arguments[key].toLowerCase();
665665
return;
666666
}
667667

‎test/unit/collection.test.js

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,48 @@ class MockTopology extends EventEmitter {
1616
}
1717

1818
describe('Collection', function() {
19-
/**
20-
* @ignore
21-
*/
22-
it('should not allow atomic operators for findOneAndReplace', {
23-
metadata: { requires: { topology: 'single' } },
24-
test: function() {
25-
const db = new Db('fakeDb', new MockTopology());
26-
const collection = db.collection('test');
27-
expect(() => {
28-
collection.findOneAndReplace({ a: 1 }, { $set: { a: 14 } });
29-
}).to.throw(/must not contain atomic operators/);
30-
}
19+
describe('findOneAndReplace()', function() {
20+
it('should throw on atomic operators in replacement document', {
21+
metadata: { requires: { topology: 'single' } },
22+
test: function() {
23+
const db = new Db('fakeDb', new MockTopology());
24+
const collection = db.collection('test');
25+
expect(() => {
26+
collection.findOneAndReplace({ a: 1 }, { $set: { a: 14 } });
27+
}).to.throw(/must not contain atomic operators/);
28+
}
29+
});
30+
31+
it('should throw if returnOriginal is specified with returnDocument as an option', {
32+
metadata: { requires: { topology: 'single' } },
33+
test: function() {
34+
const db = new Db('fakeDb', new MockTopology());
35+
const collection = db.collection('test');
36+
expect(() => {
37+
collection.findOneAndReplace(
38+
{ a: 1 },
39+
{ b: 2 },
40+
{ returnOriginal: true, returnDocument: 'before' }
41+
);
42+
}).to.throw(/returnOriginal is deprecated in favor of returnDocument/);
43+
}
44+
});
45+
});
46+
47+
describe('findOneAndUpdate()', function() {
48+
it('should throw if returnOriginal is specified with returnDocument as an option', {
49+
metadata: { requires: { topology: 'single' } },
50+
test: function() {
51+
const db = new Db('fakeDb', new MockTopology());
52+
const collection = db.collection('test');
53+
expect(() => {
54+
collection.findOneAndUpdate(
55+
{ a: 1 },
56+
{ $set: { a: 14 } },
57+
{ returnOriginal: true, returnDocument: 'before' }
58+
);
59+
}).to.throw(/returnOriginal is deprecated in favor of returnDocument/);
60+
}
61+
});
3162
});
3263
});

‎test/unit/deprecate_warning.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ describe('Deprecation Warnings', function() {
255255
deprecatedOptions: deprecatedOptions,
256256
optionsIndex: 0
257257
});
258-
expect(() => f('not-an-object')).to.not.throw();
258+
expect(() => f('not-an-object')).not.to.throw();
259259
done();
260260
});
261261
});

0 commit comments

Comments
 (0)