Skip to content

Commit 5597808

Browse files
authored
Merge pull request Automattic#15338 from Automattic/vkarpov15/Automatticgh-15335
Avoid double calling validators on paths in document arrays underneath subdocuments
2 parents 5325a85 + 46cf99b commit 5597808

File tree

2 files changed

+75
-6
lines changed

2 files changed

+75
-6
lines changed

lib/document.js

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2721,12 +2721,33 @@ function _getPathsToValidate(doc, pathsToValidate, pathsToSkip, isNestedValidate
27212721
function addToPaths(p) { paths.add(p); }
27222722

27232723
if (!isNestedValidate) {
2724-
// If we're validating a subdocument, all this logic will run anyway on the top-level document, so skip for subdocuments
2725-
const subdocs = doc.$getAllSubdocs({ useCache: true });
2724+
// If we're validating a subdocument, all this logic will run anyway on the top-level document, so skip for subdocuments.
2725+
// But only run for top-level subdocuments, because we're looking for subdocuments that are not modified at top-level but
2726+
// have a modified path. If that is the case, we will run validation on the top-level subdocument, and via that run validation
2727+
// on any subdocuments down to the modified path.
2728+
const topLevelSubdocs = [];
2729+
for (const path of Object.keys(doc.$__schema.paths)) {
2730+
const schemaType = doc.$__schema.path(path);
2731+
if (schemaType.$isSingleNested) {
2732+
const subdoc = doc.$get(path);
2733+
if (subdoc) {
2734+
topLevelSubdocs.push(subdoc);
2735+
}
2736+
} else if (schemaType.$isMongooseDocumentArray) {
2737+
const arr = doc.$get(path);
2738+
if (arr && arr.length) {
2739+
for (const subdoc of arr) {
2740+
if (subdoc) {
2741+
topLevelSubdocs.push(subdoc);
2742+
}
2743+
}
2744+
}
2745+
}
2746+
}
27262747
const modifiedPaths = doc.modifiedPaths();
2727-
for (const subdoc of subdocs) {
2748+
for (const subdoc of topLevelSubdocs) {
27282749
if (subdoc.$basePath) {
2729-
const fullPathToSubdoc = subdoc.$isSingleNested ? subdoc.$__pathRelativeToParent() : subdoc.$__fullPathWithIndexes();
2750+
const fullPathToSubdoc = subdoc.$__pathRelativeToParent();
27302751

27312752
// Remove child paths for now, because we'll be validating the whole
27322753
// subdoc.
@@ -2736,11 +2757,12 @@ function _getPathsToValidate(doc, pathsToValidate, pathsToSkip, isNestedValidate
27362757
paths.delete(fullPathToSubdoc + '.' + modifiedPath);
27372758
}
27382759

2760+
const subdocParent = subdoc.$parent();
27392761
if (doc.$isModified(fullPathToSubdoc, null, modifiedPaths) &&
27402762
// Avoid using isDirectModified() here because that does additional checks on whether the parent path
27412763
// is direct modified, which can cause performance issues re: gh-14897
2742-
!doc.$__.activePaths.getStatePaths('modify').hasOwnProperty(fullPathToSubdoc) &&
2743-
!doc.$isDefault(fullPathToSubdoc)) {
2764+
!subdocParent.$__.activePaths.getStatePaths('modify').hasOwnProperty(fullPathToSubdoc) &&
2765+
!subdocParent.$isDefault(fullPathToSubdoc)) {
27442766
paths.add(fullPathToSubdoc);
27452767

27462768
if (doc.$__.pathsToScopes == null) {

test/document.test.js

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14455,6 +14455,53 @@ describe('document', function() {
1445514455
assert.strictEqual(car.vin, undefined);
1445614456
assert.strictEqual(car.sunroof, true);
1445714457
});
14458+
14459+
it('avoids double validating document arrays underneath single nested (gh-15335)', async function() {
14460+
let arraySubdocValidateCalls = 0;
14461+
let strValidateCalls = 0;
14462+
14463+
const embeddedSchema = new mongoose.Schema({
14464+
arrObj: {
14465+
type: [{
14466+
name: {
14467+
type: String,
14468+
validate: {
14469+
validator: () => {
14470+
++arraySubdocValidateCalls;
14471+
return true;
14472+
}
14473+
}
14474+
}
14475+
}]
14476+
},
14477+
arrStr: {
14478+
type: [{
14479+
type: String,
14480+
validate: {
14481+
validator: () => {
14482+
++strValidateCalls;
14483+
return true;
14484+
}
14485+
}
14486+
}]
14487+
}
14488+
});
14489+
14490+
const TestModel = db.model('Test', new Schema({ child: embeddedSchema }));
14491+
await TestModel.create({
14492+
child: {
14493+
arrObj: [
14494+
{
14495+
name: 'arrObj'
14496+
}
14497+
],
14498+
arrStr: ['arrStr']
14499+
}
14500+
});
14501+
assert.strictEqual(arraySubdocValidateCalls, 1);
14502+
assert.strictEqual(strValidateCalls, 1);
14503+
14504+
});
1445814505
});
1445914506

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

0 commit comments

Comments
 (0)