Skip to content

Commit 0a1cd39

Browse files
authored
Merge pull request #15257 from Automattic/vkarpov15/gh-15255
perf(document): only call `undoReset()` 1x/document
2 parents d48bc5f + 9bb9ce7 commit 0a1cd39

File tree

2 files changed

+46
-2
lines changed

2 files changed

+46
-2
lines changed

lib/document.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3553,8 +3553,10 @@ Document.prototype.$__undoReset = function $__undoReset() {
35533553
}
35543554
}
35553555

3556-
for (const subdoc of this.$getAllSubdocs()) {
3557-
subdoc.$__undoReset();
3556+
if (!this.$isSubdocument) {
3557+
for (const subdoc of this.$getAllSubdocs()) {
3558+
subdoc.$__undoReset();
3559+
}
35583560
}
35593561
};
35603562

test/document.test.js

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const ArraySubdocument = require('../lib/types/arraySubdocument');
1212
const Query = require('../lib/query');
1313
const assert = require('assert');
1414
const idGetter = require('../lib/helpers/schema/idGetter');
15+
const sinon = require('sinon');
1516
const util = require('./util');
1617
const utils = require('../lib/utils');
1718

@@ -14296,6 +14297,47 @@ describe('document', function() {
1429614297

1429714298
delete mongoose.Schema.Types.CustomType;
1429814299
});
14300+
14301+
it('handles undoReset() on deep recursive subdocuments (gh-15255)', async function() {
14302+
const RecursiveSchema = new mongoose.Schema({});
14303+
14304+
const s = [RecursiveSchema];
14305+
RecursiveSchema.path('nested', s);
14306+
14307+
const generateRecursiveDocument = (depth, curr = 0) => {
14308+
return {
14309+
name: `Document of depth ${curr}`,
14310+
nested: depth > 0 ? new Array(2).fill().map(() => generateRecursiveDocument(depth - 1, curr + 1)) : [],
14311+
__v: 5
14312+
};
14313+
};
14314+
const TestModel = db.model('Test', RecursiveSchema);
14315+
const data = generateRecursiveDocument(10);
14316+
const doc = new TestModel(data);
14317+
await doc.save();
14318+
14319+
sinon.spy(Document.prototype, '$__undoReset');
14320+
14321+
try {
14322+
const d = await TestModel.findById(doc._id);
14323+
d.increment();
14324+
d.data = 'asd';
14325+
// Force a version error by updating the document directly
14326+
await TestModel.collection.updateOne({ _id: doc._id }, { $inc: { __v: 1 } });
14327+
const err = await d.save().then(() => null, err => err);
14328+
assert.ok(err);
14329+
assert.equal(err.name, 'VersionError');
14330+
// `$__undoReset()` should be called 1x per subdoc, plus 1x for top-level doc. Without fix for gh-15255,
14331+
// this would fail because `$__undoReset()` is called nearly 700k times for only 2046 subdocs
14332+
assert.strictEqual(Document.prototype.$__undoReset.getCalls().length, d.$getAllSubdocs().length + 1);
14333+
assert.ok(Document.prototype.$__undoReset.getCalls().find(call => call.thisValue === d), 'top level doc was not reset');
14334+
for (const subdoc of d.$getAllSubdocs()) {
14335+
assert.ok(Document.prototype.$__undoReset.getCalls().find(call => call.thisValue === subdoc), `${subdoc.name} was not reset`);
14336+
}
14337+
} finally {
14338+
sinon.restore();
14339+
}
14340+
});
1429914341
});
1430014342

1430114343
describe('Check if instance function that is supplied in schema option is available', function() {

0 commit comments

Comments
 (0)