Skip to content

Commit

Permalink
Merge pull request #13843 from Automattic/vkarpov15/gh-13782
Browse files Browse the repository at this point in the history
BREAKING CHANGE: apply minimize by default when updating document
  • Loading branch information
vkarpov15 committed Sep 26, 2023
2 parents 2348b7a + 8527c8d commit d5e9176
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 47 deletions.
37 changes: 1 addition & 36 deletions lib/document.js
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
@@ -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
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
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

0 comments on commit d5e9176

Please sign in to comment.