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

BREAKING CHANGE: apply minimize by default when updating document #13843

Merged
merged 3 commits into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 1 addition & 36 deletions lib/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const isExclusive = require('./helpers/projection/isExclusive');
const inspect = require('util').inspect;
const internalToObjectOptions = require('./options').internalToObjectOptions;
const markArraySubdocsPopulated = require('./helpers/populate/markArraySubdocsPopulated');
const minimize = require('./helpers/minimize');
const mpath = require('mpath');
const queryhelpers = require('./queryHelpers');
const utils = require('./utils');
Expand Down Expand Up @@ -3938,42 +3939,6 @@ Document.prototype.toObject = function(options) {
return this.$toObject(options);
};

/**
* Minimizes an object, removing undefined values and empty objects
*
* @param {Object} object to minimize
* @return {Object}
* @api private
*/

function minimize(obj) {
const keys = Object.keys(obj);
let i = keys.length;
let hasKeys;
let key;
let val;

while (i--) {
key = keys[i];
val = obj[key];

if (utils.isPOJO(val)) {
obj[key] = minimize(val);
}

if (undefined === obj[key]) {
delete obj[key];
continue;
}

hasKeys = true;
}

return hasKeys
? obj
: undefined;
}

/*!
* Applies virtuals properties to `json`.
*/
Expand Down
41 changes: 41 additions & 0 deletions lib/helpers/minimize.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
'use strict';

const { isPOJO } = require('../utils');

module.exports = minimize;

/**
* Minimizes an object, removing undefined values and empty objects
*
* @param {Object} object to minimize
* @return {Object|undefined}
* @api private
*/

function minimize(obj) {
const keys = Object.keys(obj);
let i = keys.length;
let hasKeys;
let key;
let val;

while (i--) {
key = keys[i];
val = obj[key];

if (isPOJO(val)) {
obj[key] = minimize(val);
}

if (undefined === obj[key]) {
delete obj[key];
continue;
}

hasKeys = true;
}

return hasKeys
? obj
: undefined;
}
33 changes: 25 additions & 8 deletions lib/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const STATES = require('./connectionState');
const util = require('util');
const utils = require('./utils');
const MongooseBulkWriteError = require('./error/bulkWriteError');
const minimize = require('./helpers/minimize');

const VERSION_WHERE = 1;
const VERSION_INC = 2;
Expand Down Expand Up @@ -342,7 +343,19 @@ Model.prototype.$__handleSave = function(options, callback) {
}

_applyCustomWhere(this, where);
this[modelCollectionSymbol].updateOne(where, delta[1], saveOptions).then(

const update = delta[1];
if (this.$__schema.options.minimize) {
minimize(update);
// minimize might leave us with an empty object, which would
// lead to MongoDB throwing a "Update document requires atomic operators" error
if (Object.keys(update).length === 0) {
handleEmptyUpdate.call(this);
return;
}
}

this[modelCollectionSymbol].updateOne(where, update, saveOptions).then(
ret => {
ret.$where = where;
callback(null, ret);
Expand All @@ -354,6 +367,17 @@ Model.prototype.$__handleSave = function(options, callback) {
}
);
} else {
handleEmptyUpdate.call(this);
return;
}

// store the modified paths before the document is reset
this.$__.modifiedPaths = this.modifiedPaths();
this.$__reset();

_setIsNew(this, false);

function handleEmptyUpdate() {
const optionsWithCustomValues = Object.assign({}, options, saveOptions);
const where = this.$__where();
const optimisticConcurrency = this.$__schema.options.optimisticConcurrency;
Expand All @@ -370,14 +394,7 @@ Model.prototype.$__handleSave = function(options, callback) {
callback(null, { $where: where, matchedCount });
})
.catch(callback);
return;
}

// store the modified paths before the document is reset
this.$__.modifiedPaths = this.modifiedPaths();
this.$__reset();

_setIsNew(this, false);
};

/*!
Expand Down
6 changes: 3 additions & 3 deletions test/document.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3243,7 +3243,7 @@ describe('document', function() {
field1: { type: Number, default: 1 }
}
}
});
}, { minimize: false });

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

Expand All @@ -3262,7 +3262,7 @@ describe('document', function() {
field1: { type: Number, default: 1 }
}
}
});
}, { minimize: false });

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

Expand Down Expand Up @@ -5002,7 +5002,7 @@ describe('document', function() {
}).
then(function(doc) {
doc.child = {};
return doc.save();
return Parent.updateOne({ _id: doc._id }, { $set: { child: {} } }, { minimize: false });
}).
then(function() {
return Parent.findOne();
Expand Down