Skip to content

Commit

Permalink
Merge pull request #13989 from Automattic/vkarpov15/gh-13578
Browse files Browse the repository at this point in the history
Remove `overwrite` option for `updateOne()`, `findOneAndUpdate()`, etc.
  • Loading branch information
vkarpov15 authored Oct 24, 2023
2 parents b630afb + 30888e3 commit f923f6c
Show file tree
Hide file tree
Showing 12 changed files with 28 additions and 160 deletions.
16 changes: 13 additions & 3 deletions docs/migrating_to_8.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ If you're still on Mongoose 6.x or earlier, please read the [Mongoose 6.x to 7.x
* [`null` is valid for non-required string enums](#null-is-valid-for-non-required-string-enums)
* [Apply minimize when `save()` updates an existing document](#apply-minimize-when-save-updates-an-existing-document)
* [Apply base schema paths before discriminator paths](#apply-base-schema-paths-before-discriminator-paths)
* [Removed `overwrite` option for `findOneAndUpdate()`](#removed-overwrite-option-for-findoneandupdate)
* [Changed behavior for `findOneAndUpdate()` with `orFail()` and upsert](#changed-behavior-for-findoneandupdate-with-orfail-and-upsert)
* [`create()` waits until all saves are done before throwing any error](#create-waits-until-all-saves-are-done-before-throwing-any-error)
* [`Model.validate()` returns copy of object](#model-validate-returns-copy-of-object)
Expand Down Expand Up @@ -133,8 +134,8 @@ rawDoc.nested; // undefined in Mongoose 8, {} in Mongoose 7

<h2 id="apply-base-schema-paths-before-discriminator-paths"><a href="#apply-base-schema-paths-before-discriminator-paths">Apply base schema paths before discriminator paths</a></h2>

This means that, in Mongoose 8, getters and setters on discriminator paths run _after_ getters and setters on base paths.
In Mongoose 7, getters and setters on discriminator paths ran _before_ getters and setters on base paths.
This means that, in Mongoose 8, getters and setters on discriminator paths run *after* getters and setters on base paths.
In Mongoose 7, getters and setters on discriminator paths ran *before* getters and setters on base paths.

```javascript

Expand Down Expand Up @@ -165,6 +166,15 @@ const doc = new D({ name: 'test', otherProp: 'test' });
console.log(doc.toObject({ getters: true }));
```

<h2 id="removed-overwrite-option-for-findoneandupdate"><a href="#removed-overwrite-option-for-findoneandupdate">Removed <code>overwrite</code> option for <code>findOneAndUpdate()</code></a></h2>

Mongoose 7 and earlier supported an `overwrite` option for `findOneAndUpdate()`, `updateOne()`, and `update()`.
Before Mongoose 7, `overwrite` would skip wrapping the `update` parameter in `$set`, which meant that `findOneAndUpdate()` and `update()` would overwrite the matched document.
In Mongoose 7, setting `overwrite` would convert `findOneAndUpdate()` to `findOneAndReplace()` and `updateOne()` to `replaceOne()` to retain backwards compatibility.

In Mongoose 8, the `overwrite` option is no longer supported.
If you want to overwrite the entire document, use `findOneAndReplace()` or `replaceOne()`.

<h2 id="changed-behavior-for-findoneandupdate-with-orfail-and-upsert"><a href="#changed-behavior-for-findoneandupdate-with-orfail-and-upsert">Changed behavior for <code>findOneAndUpdate()</code> with <code>orFail()</code> and upsert</a></h2>

In Mongoose 7, `findOneAndUpdate(filter, update, { upsert: true }).orFail()` would throw a `DocumentNotFoundError` if a new document was upserted.
Expand Down Expand Up @@ -253,4 +263,4 @@ const schema = new Schema<User>({

// Works in Mongoose 8. Compile error in Mongoose 7.
const names: string[] = await MyModel.distinct('name');
```
```
2 changes: 0 additions & 2 deletions lib/helpers/model/castBulkWrite.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ module.exports = function castBulkWrite(originalModel, op, options) {

op['updateOne']['update'] = castUpdate(model.schema, op['updateOne']['update'], {
strict: strict,
overwrite: false,
upsert: op['updateOne'].upsert
}, model, op['updateOne']['filter']);
} catch (error) {
Expand Down Expand Up @@ -157,7 +156,6 @@ module.exports = function castBulkWrite(originalModel, op, options) {

op['updateMany']['update'] = castUpdate(model.schema, op['updateMany']['update'], {
strict: strict,
overwrite: false,
upsert: op['updateMany'].upsert
}, model, op['updateMany']['filter']);
} catch (error) {
Expand Down
13 changes: 3 additions & 10 deletions lib/helpers/query/castUpdate.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ const mongodbUpdateOperators = new Set([
* @param {Schema} schema
* @param {Object} obj
* @param {Object} [options]
* @param {Boolean} [options.overwrite] defaults to false
* @param {Boolean|String} [options.strict] defaults to true
* @param {Query} context passed to setters
* @return {Boolean} true iff the update is non-empty
Expand All @@ -61,7 +60,7 @@ module.exports = function castUpdate(schema, obj, options, context, filter) {
return obj;
}

if (options.upsert && !options.overwrite) {
if (options.upsert) {
moveImmutableProperties(schema, obj, context);
}

Expand All @@ -70,13 +69,11 @@ module.exports = function castUpdate(schema, obj, options, context, filter) {
const ret = {};
let val;
let hasDollarKey = false;
const overwrite = options.overwrite;

filter = filter || {};
while (i--) {
const op = ops[i];
// if overwrite is set, don't do any of the special $set stuff
if (!mongodbUpdateOperators.has(op) && !overwrite) {
if (!mongodbUpdateOperators.has(op)) {
// fix up $set sugar
if (!ret.$set) {
if (obj.$set) {
Expand Down Expand Up @@ -106,10 +103,8 @@ module.exports = function castUpdate(schema, obj, options, context, filter) {
if (val &&
typeof val === 'object' &&
!Buffer.isBuffer(val) &&
(!overwrite || mongodbUpdateOperators.has(op))) {
mongodbUpdateOperators.has(op)) {
walkUpdatePath(schema, val, op, options, context, filter);
} else if (overwrite && ret && typeof ret === 'object') {
walkUpdatePath(schema, ret, '$set', options, context, filter);
} else {
const msg = 'Invalid atomic update value for ' + op + '. '
+ 'Expected an object, received ' + typeof val;
Expand Down Expand Up @@ -239,7 +234,6 @@ function walkUpdatePath(schema, obj, op, options, context, filter, pref) {
}

if (op !== '$setOnInsert' &&
!options.overwrite &&
handleImmutable(schematype, strict, obj, key, prefix + key, context)) {
continue;
}
Expand Down Expand Up @@ -334,7 +328,6 @@ function walkUpdatePath(schema, obj, op, options, context, filter, pref) {

// You can use `$setOnInsert` with immutable keys
if (op !== '$setOnInsert' &&
!options.overwrite &&
handleImmutable(schematype, strict, obj, key, prefix + key, context)) {
continue;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/helpers/timestamps/setupTimestamps.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ module.exports = function setupTimestamps(schema, timestamps) {
updatedAt,
this.getUpdate(),
this._mongooseOptions,
this.schema
replaceOps.has(this.op)
);
applyTimestampsToChildren(now, this.getUpdate(), this.model.schema);
next();
Expand Down
5 changes: 2 additions & 3 deletions lib/helpers/update/applyTimestampsToUpdate.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@ module.exports = applyTimestampsToUpdate;
* ignore
*/

function applyTimestampsToUpdate(now, createdAt, updatedAt, currentUpdate, options) {
function applyTimestampsToUpdate(now, createdAt, updatedAt, currentUpdate, options, isReplace) {
const updates = currentUpdate;
let _updates = updates;
const overwrite = get(options, 'overwrite', false);
const timestamps = get(options, 'timestamps', true);

// Support skipping timestamps at the query level, see gh-6980
Expand All @@ -26,7 +25,7 @@ function applyTimestampsToUpdate(now, createdAt, updatedAt, currentUpdate, optio
const skipCreatedAt = timestamps != null && timestamps.createdAt === false;
const skipUpdatedAt = timestamps != null && timestamps.updatedAt === false;

if (overwrite) {
if (isReplace) {
if (currentUpdate && currentUpdate.$set) {
currentUpdate = currentUpdate.$set;
updates.$set = {};
Expand Down
5 changes: 0 additions & 5 deletions lib/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -2394,7 +2394,6 @@ Model.$where = function $where() {
* @param {ClientSession} [options.session=null] The session associated with this query. See [transactions docs](https://mongoosejs.com/docs/transactions.html).
* @param {Boolean|String} [options.strict] overwrites the schema's [strict mode option](https://mongoosejs.com/docs/guide.html#strict)
* @param {Boolean} [options.timestamps=null] If set to `false` and [schema-level timestamps](https://mongoosejs.com/docs/guide.html#timestamps) are enabled, skip timestamps for this update. Note that this allows you to overwrite timestamps. Does nothing if schema-level timestamps are not set.
* @param {Boolean} [options.overwrite=false] If set to `true`, Mongoose will convert this `findOneAndUpdate()` to a `findOneAndReplace()`. This option is deprecated and only supported for backwards compatiblity.
* @param {Boolean} [options.upsert=false] if true, and no documents found, insert a new document
* @param {Object|String|String[]} [options.projection=null] optional fields to return, see [`Query.prototype.select()`](https://mongoosejs.com/docs/api/query.html#Query.prototype.select())
* @param {Boolean} [options.new=false] if true, return the modified document rather than the original
Expand Down Expand Up @@ -2469,9 +2468,6 @@ Model.findOneAndUpdate = function(conditions, update, options) {
* // is sent as
* Model.findByIdAndUpdate(id, { $set: { name: 'jason bourne' }}, options)
*
* This helps prevent accidentally overwriting your document with `{ name: 'jason bourne' }`.
* To prevent this behaviour, see the `overwrite` option
*
* #### Note:
*
* `findOneAndX` and `findByIdAndX` functions support limited validation. You can
Expand All @@ -2492,7 +2488,6 @@ Model.findOneAndUpdate = function(conditions, update, options) {
* @param {ClientSession} [options.session=null] The session associated with this query. See [transactions docs](https://mongoosejs.com/docs/transactions.html).
* @param {Boolean|String} [options.strict] overwrites the schema's [strict mode option](https://mongoosejs.com/docs/guide.html#strict)
* @param {Boolean} [options.timestamps=null] If set to `false` and [schema-level timestamps](https://mongoosejs.com/docs/guide.html#timestamps) are enabled, skip timestamps for this update. Note that this allows you to overwrite timestamps. Does nothing if schema-level timestamps are not set.
* @param {Boolean} [options.overwrite=false] If set to `true`, Mongoose will convert this `findByIdAndUpdate()` to a `findByIdAndReplace()`. This option is deprecated and only supported for backwards compatiblity.
* @param {Object|String} [options.sort] if multiple docs are found by the conditions, sets the sort order to choose which doc to update.
* @param {Boolean} [options.runValidators] if true, runs [update validators](https://mongoosejs.com/docs/validation.html#update-validators) on this command. Update validators validate the update operation against the model's schema
* @param {Boolean} [options.setDefaultsOnInsert=true] If `setDefaultsOnInsert` and `upsert` are true, mongoose will apply the [defaults](https://mongoosejs.com/docs/defaults.html) specified in the model's schema if a new document is created
Expand Down
46 changes: 4 additions & 42 deletions lib/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const castUpdate = require('./helpers/query/castUpdate');
const clone = require('./helpers/clone');
const completeMany = require('./helpers/query/completeMany');
const getDiscriminatorByValue = require('./helpers/discriminator/getDiscriminatorByValue');
const hasDollarKeys = require('./helpers/query/hasDollarKeys');
const helpers = require('./queryHelpers');
const immediate = require('./helpers/immediate');
const internalToObjectOptions = require('./options').internalToObjectOptions;
Expand Down Expand Up @@ -1619,10 +1618,6 @@ Query.prototype.setOptions = function(options, overwrite) {
this._mongooseOptions.sanitizeFilter = options.sanitizeFilter;
delete options.sanitizeFilter;
}
if ('overwrite' in options) {
this._mongooseOptions.overwrite = options.overwrite;
delete options.overwrite;
}
if ('timestamps' in options) {
this._mongooseOptions.timestamps = options.timestamps;
delete options.timestamps;
Expand Down Expand Up @@ -1670,15 +1665,6 @@ Query.prototype.setOptions = function(options, overwrite) {
return this;
};

/*!
* ignore
*/

const printOverwriteDeprecationWarning = util.deprecate(
function printOverwriteDeprecationWarning() {},
'The `overwrite` option for `findOneAndUpdate()` is deprecated. use `findOneAndReplace()` instead.'
);

/**
* Sets the [`explain` option](https://www.mongodb.com/docs/manual/reference/method/cursor.explain/),
* which makes this query return detailed execution stats instead of the actual
Expand Down Expand Up @@ -1898,11 +1884,6 @@ Query.prototype._updateForExec = function() {
while (i--) {
const op = ops[i];

if (this._mongooseOptions.overwrite) {
ret[op] = update[op];
continue;
}

if ('$' !== op[0]) {
// fix up $set sugar
if (!ret.$set) {
Expand Down Expand Up @@ -3259,16 +3240,6 @@ Query.prototype.findOneAndUpdate = function(filter, doc, options) {
*/

Query.prototype._findOneAndUpdate = async function _findOneAndUpdate() {
// For backwards compability with Mongoose 6 re: #13550

if (this._mongooseOptions.overwrite != null) {
printOverwriteDeprecationWarning();
}
if (this._mongooseOptions.overwrite) {
this.op = 'findOneAndReplace';
return this._findOneAndReplace();
}

this._castConditions();

_castArrayFilters(this);
Expand All @@ -3287,7 +3258,7 @@ Query.prototype._findOneAndUpdate = async function _findOneAndUpdate() {
convertNewToReturnDocument(options);
this._applyTranslateAliases(options);

this._update = this._castUpdate(this._update, false);
this._update = this._castUpdate(this._update);

const _opts = Object.assign({}, options, {
setDefaultsOnInsert: this._mongooseOptions.setDefaultsOnInsert
Expand Down Expand Up @@ -3520,7 +3491,6 @@ Query.prototype.findOneAndReplace = function(filter, replacement, options) {
options.returnOriginal = returnOriginal;
}
this.setOptions(options);
this.setOptions({ overwrite: true });

return this;
};
Expand Down Expand Up @@ -3755,16 +3725,11 @@ async function _updateThunk(op) {
this._applyTranslateAliases(options);

this._update = clone(this._update, options);
const isOverwriting = this._mongooseOptions.overwrite && !hasDollarKeys(this._update);
const isOverwriting = op === 'replaceOne';
if (isOverwriting) {
if (op === 'updateOne' || op === 'updateMany') {
throw new MongooseError('The MongoDB server disallows ' +
'overwriting documents using `' + op + '`. See: ' +
'https://mongoosejs.com/docs/deprecations.html#update');
}
this._update = new this.model(this._update, null, true);
} else {
this._update = this._castUpdate(this._update, this._mongooseOptions.overwrite);
this._update = this._castUpdate(this._update);

if (this._update == null || Object.keys(this._update).length === 0) {
return { acknowledged: false };
Expand Down Expand Up @@ -4065,7 +4030,6 @@ Query.prototype.replaceOne = function(conditions, doc, options, callback) {
callback = undefined;
}

this.setOptions({ overwrite: true });
return _update(this, 'replaceOne', conditions, doc, options, callback);
};

Expand Down Expand Up @@ -4563,15 +4527,14 @@ Query.prototype.post = function(fn) {
* Casts obj for an update command.
*
* @param {Object} obj
* @param {Boolean} overwrite
* @return {Object} obj after casting its values
* @method _castUpdate
* @memberOf Query
* @instance
* @api private
*/

Query.prototype._castUpdate = function _castUpdate(obj, overwrite) {
Query.prototype._castUpdate = function _castUpdate(obj) {
let schema = this.schema;

const discriminatorKey = schema.options.discriminatorKey;
Expand Down Expand Up @@ -4605,7 +4568,6 @@ Query.prototype._castUpdate = function _castUpdate(obj, overwrite) {
}

return castUpdate(schema, obj, {
overwrite: overwrite,
strict: this._mongooseOptions.strict,
upsert: upsert,
arrayFilters: this.options.arrayFilters,
Expand Down
4 changes: 2 additions & 2 deletions test/document.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1985,7 +1985,7 @@ describe('document', function() {

const err = await person.save().then(() => null, err => err);
assert.equal(err instanceof DocumentNotFoundError, true);
assert.equal(err.message, `No document found for query "{ _id: new ObjectId("${person._id}") }" on model "Person"`);
assert.equal(err.message, `No document found for query "{ _id: new ObjectId('${person._id}') }" on model "Person"`);
});

it('saving a document when version bump required, throws a VersionError when document is not found (gh-10974)', async function() {
Expand Down Expand Up @@ -2020,7 +2020,7 @@ describe('document', function() {
}
catch (err) {
assert.equal(err instanceof DocumentNotFoundError, true);
assert.equal(err.message, `No document found for query "{ _id: new ObjectId("${person._id}") }" on model "Person"`);
assert.equal(err.message, `No document found for query "{ _id: new ObjectId('${person._id}') }" on model "Person"`);
threw = true;
}

Expand Down
Loading

0 comments on commit f923f6c

Please sign in to comment.