Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove overwrite option for updateOne(), findOneAndUpdate(), etc. #13989

Merged
merged 10 commits into from
Oct 24, 2023
2 changes: 0 additions & 2 deletions lib/helpers/model/castBulkWrite.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,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 @@ -140,7 +139,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 @@ -238,7 +233,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 @@ -333,7 +327,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
15 changes: 3 additions & 12 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 @@ -2453,14 +2452,10 @@ function _decorateUpdateWithVersionKey(update, options, versionKey) {

const updatedPaths = modifiedPaths(update);
if (!updatedPaths[versionKey]) {
if (options.overwrite) {
update[versionKey] = 0;
} else {
if (!update.$setOnInsert) {
update.$setOnInsert = {};
}
update.$setOnInsert[versionKey] = 0;
if (!update.$setOnInsert) {
update.$setOnInsert = {};
}
update.$setOnInsert[versionKey] = 0;
}
}

Expand Down Expand Up @@ -2492,9 +2487,6 @@ function _decorateUpdateWithVersionKey(update, options, versionKey) {
* // 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 @@ -2515,7 +2507,6 @@ function _decorateUpdateWithVersionKey(update, options, versionKey) {
* @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
68 changes: 0 additions & 68 deletions test/model.updateOne.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -874,22 +874,6 @@ describe('model: updateOne:', function() {
});
});

it('works with $set and overwrite (gh-2515)', async function() {
const schema = new Schema({ breakfast: String });
const M = db.model('Test', schema);

let doc = await M.create({ breakfast: 'bacon' });
await M.updateOne(
{ _id: doc._id },
{ $set: { breakfast: 'eggs' } },
{ overwrite: true }
);

doc = await M.findOne({ _id: doc._id });
assert.equal(doc.breakfast, 'eggs');

});

it('successfully casts set with nested mixed objects (gh-2796)', async function() {
const schema = new Schema({ breakfast: {} });
const M = db.model('Test', schema);
Expand Down Expand Up @@ -1246,33 +1230,6 @@ describe('model: updateOne:', function() {
assert.equal(doc.name, 'Val');
});

it('casting $push with overwrite (gh-3564)', async function() {
const schema = mongoose.Schema({
topicId: Number,
name: String,
followers: [Number]
});

let doc = {
topicId: 100,
name: 'name',
followers: [500]
};

const M = db.model('Test', schema);

await M.create(doc);

const update = { $push: { followers: 200 } };
const opts = { overwrite: true, new: true, upsert: false, multi: false };

await M.updateOne({ topicId: doc.topicId }, update, opts);
doc = await M.findOne({ topicId: doc.topicId });
assert.equal(doc.name, 'name');
assert.deepEqual(doc.followers.toObject(), [500, 200]);

});

it('$push with buffer doesnt throw error (gh-3890)', async function() {
const InfoSchema = new Schema({
prop: { type: Buffer }
Expand Down Expand Up @@ -1747,31 +1704,6 @@ describe('model: updateOne:', function() {
assert.deepEqual(doc.profiles[0], { rules: {} });
});

it('with overwrite and upsert (gh-4749) (gh-5631)', function() {
const schema = new Schema({
name: String,
meta: { age: { type: Number } }
});
const User = db.model('User', schema);

const filter = { name: 'Bar' };
const update = { name: 'Bar', meta: { age: 33 } };
const options = { overwrite: true, upsert: true };
const q2 = User.updateOne(filter, update, options);
assert.deepEqual(q2.getUpdate(), {
__v: 0,
meta: { age: 33 },
name: 'Bar'
});

const q3 = User.findOneAndUpdate(filter, update, options);
assert.deepEqual(q3.getUpdate(), {
__v: 0,
meta: { age: 33 },
name: 'Bar'
});
});

it('findOneAndUpdate with nested arrays (gh-5032)', async function() {
const schema = Schema({
name: String,
Expand Down