Skip to content

Commit 40626d1

Browse files
authored
Merge pull request #11902 from Automattic/gh-11770
fix: make `doValidate()` on document array elements run validation on the whole subdoc
2 parents 69dc0ee + 379c459 commit 40626d1

File tree

7 files changed

+72
-24
lines changed

7 files changed

+72
-24
lines changed

lib/document.js

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2486,7 +2486,7 @@ function _getPathsToValidate(doc) {
24862486
const fullPathToSubdoc = subdoc.$__fullPathWithIndexes();
24872487

24882488
for (const p of paths) {
2489-
if (p === null || p.startsWith(fullPathToSubdoc + '.')) {
2489+
if (p == null || p.startsWith(fullPathToSubdoc + '.')) {
24902490
paths.delete(p);
24912491
}
24922492
}
@@ -2507,6 +2507,14 @@ function _getPathsToValidate(doc) {
25072507
continue;
25082508
}
25092509

2510+
if (_pathType.$isMongooseDocumentArray) {
2511+
for (const p of paths) {
2512+
if (p == null || p.startsWith(_pathType.path + '.')) {
2513+
paths.delete(p);
2514+
}
2515+
}
2516+
}
2517+
25102518
// Optimization: if primitive path with no validators, or array of primitives
25112519
// with no validators, skip validating this path entirely.
25122520
if (!_pathType.caster && _pathType.validators.length === 0) {
@@ -3145,16 +3153,15 @@ Document.prototype.$__reset = function reset() {
31453153
if (subdoc.$isDocumentArrayElement) {
31463154
if (!resetArrays.has(subdoc.parentArray())) {
31473155
const array = subdoc.parentArray();
3148-
// Mark path to array as init for gh-6818
3149-
this.$__.activePaths.init(fullPathWithIndexes.replace(/\.\d+$/, '').slice(-subdoc.$basePath - 1));
3156+
this.$__.activePaths.clearPath(fullPathWithIndexes.replace(/\.\d+$/, '').slice(-subdoc.$basePath - 1));
31503157
array[arrayAtomicsBackupSymbol] = array[arrayAtomicsSymbol];
31513158
array[arrayAtomicsSymbol] = {};
31523159

31533160
resetArrays.add(array);
31543161
}
31553162
} else {
31563163
if (subdoc.$parent() === this) {
3157-
this.$__.activePaths.init(subdoc.$basePath);
3164+
this.$__.activePaths.clearPath(subdoc.$basePath);
31583165
} else if (subdoc.$parent() != null && subdoc.$parent().$isSubdocument) {
31593166
// If map path underneath subdocument, may end up with a case where
31603167
// map path is modified but parent still needs to be reset. See gh-10295

lib/helpers/updateValidators.js

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -125,26 +125,19 @@ module.exports = function(query, schema, castedDoc, options, callback) {
125125
validatorsToExecute.push(function(callback) {
126126
schemaPath.doValidate(v, function(err) {
127127
if (err) {
128-
err.path = updates[i];
129-
validationErrors.push(err);
130-
return callback(null);
131-
}
132-
133-
v.validate(function(err) {
134-
if (err) {
135-
if (err.errors) {
136-
for (const key of Object.keys(err.errors)) {
137-
const _err = err.errors[key];
138-
_err.path = updates[i] + '.' + key;
139-
validationErrors.push(_err);
140-
}
141-
} else {
142-
err.path = updates[i];
143-
validationErrors.push(err);
128+
if (err.errors) {
129+
for (const key of Object.keys(err.errors)) {
130+
const _err = err.errors[key];
131+
_err.path = updates[i] + '.' + key;
132+
validationErrors.push(_err);
144133
}
134+
} else {
135+
err.path = updates[i];
136+
validationErrors.push(err);
145137
}
146-
callback(null);
147-
});
138+
}
139+
140+
return callback(null);
148141
}, context, { updateValidator: true });
149142
});
150143
} else {

lib/model.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4281,7 +4281,7 @@ Model.validate = function validate(obj, pathsToValidate, context, callback) {
42814281

42824282
for (const path of paths) {
42834283
const schemaType = schema.path(path);
4284-
if (!schemaType || !schemaType.$isMongooseArray) {
4284+
if (!schemaType || !schemaType.$isMongooseArray || schemaType.$isMongooseDocumentArray) {
42854285
continue;
42864286
}
42874287

lib/schema/documentarray.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const EventEmitter = require('events').EventEmitter;
1010
const SchemaDocumentArrayOptions =
1111
require('../options/SchemaDocumentArrayOptions');
1212
const SchemaType = require('../schematype');
13+
const SubdocumentPath = require('./SubdocumentPath');
1314
const discriminator = require('../helpers/model/discriminator');
1415
const handleIdOption = require('../helpers/schema/handleIdOption');
1516
const handleSpreadDoc = require('../helpers/document/handleSpreadDoc');
@@ -75,6 +76,15 @@ function DocumentArrayPath(key, schema, options, schemaOptions) {
7576
this.$embeddedSchemaType.cast = function(value, doc, init) {
7677
return parentSchemaType.cast(value, doc, init)[0];
7778
};
79+
this.$embeddedSchemaType.doValidate = function(value, fn, scope, options) {
80+
const Constructor = getConstructor(this.caster, value);
81+
82+
if (value && !(value instanceof Constructor)) {
83+
value = new Constructor(value, scope, null, null, options && options.index != null ? options.index : null);
84+
}
85+
86+
return SubdocumentPath.prototype.doValidate.call(this, value, fn, scope, options);
87+
};
7888
this.$embeddedSchemaType.$isMongooseDocumentArrayElement = true;
7989
this.$embeddedSchemaType.caster = this.Constructor;
8090
this.$embeddedSchemaType.schema = this.schema;

lib/statemachine.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,19 @@ StateMachine.prototype.clear = function clear(state) {
9595
}
9696
};
9797

98+
/*!
99+
* ignore
100+
*/
101+
102+
StateMachine.prototype.clearPath = function clearPath(path) {
103+
const state = this.paths[path];
104+
if (!state) {
105+
return;
106+
}
107+
delete this.paths[path];
108+
delete this.states[state][path];
109+
};
110+
98111
/*!
99112
* Checks to see if at least one path is in the states passed in via `arguments`
100113
* e.g., this.some('required', 'inited')

test/document.test.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8365,7 +8365,6 @@ describe('document', function() {
83658365
});
83668366
const Organization = db.model('Test', organizationSchema);
83678367

8368-
83698368
const org = new Organization();
83708369
org.set('name', 'MongoDB');
83718370

test/schema.documentarray.test.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,4 +126,30 @@ describe('schema.documentarray', function() {
126126
const value = testSchema.path('comments').getDefault();
127127
assert.strictEqual(value, null);
128128
});
129+
130+
it('doValidate() validates entire subdocument (gh-11770)', async function() {
131+
mongoose.deleteModel(/Test/);
132+
133+
const testSchema = new Schema({
134+
comments: [{
135+
text: {
136+
type: String,
137+
required: true
138+
}
139+
}]
140+
});
141+
const TestModel = mongoose.model('Test', testSchema);
142+
const testDoc = new TestModel();
143+
144+
const err = await new Promise((resolve, reject) => {
145+
testSchema.path('comments').$embeddedSchemaType.doValidate({}, err => {
146+
if (err != null) {
147+
return reject(err);
148+
}
149+
resolve();
150+
}, testDoc.comments, { index: 1 });
151+
}).then(() => null, err => err);
152+
assert.equal(err.name, 'ValidationError');
153+
assert.equal(err.message, 'Validation failed: text: Path `text` is required.');
154+
});
129155
});

0 commit comments

Comments
 (0)