fix: make doValidate() on document array elements run validation on the whole subdoc
#11902
Conversation
… the whole subdoc Fix #11770
lib/document.js
Outdated
| @@ -2505,6 +2505,14 @@ function _getPathsToValidate(doc) { | |||
| continue; | |||
| } | |||
|
|
|||
| if (_pathType.$isMongooseDocumentArray) { | |||
| for (const p of paths) { | |||
| if (p === null || p.startsWith(_pathType.path + '.')) { | |||
There was a problem hiding this comment.
Is strict equality in p === null intended, or did you mean p == null?
There was a problem hiding this comment.
In ine 2487 we also have strill null check
|
LGTM, thanks. 👍 Frankly, I've always found it difficult to tell the difference between Subdocument, embedded documents, and arrays of documents. |
lib/document.js
Outdated
| @@ -2505,6 +2505,14 @@ function _getPathsToValidate(doc) { | |||
| continue; | |||
| } | |||
|
|
|||
| if (_pathType.$isMongooseDocumentArray) { | |||
| for (const p of paths) { | |||
| if (p === null || p.startsWith(_pathType.path + '.')) { | |||
There was a problem hiding this comment.
In ine 2487 we also have strill null check
lib/helpers/updateValidators.js
Outdated
| } else { | ||
| err.path = updates[i]; | ||
| validationErrors.push(err); | ||
| } | ||
| return callback(null); |
There was a problem hiding this comment.
I guess we can remove this line, as we anyway go to line 141?
|
@AbdelrahmanHafez we did some work to consolidate array subdocuments and single nested subdocuments for 6.0. Now, |
Fix #11770
Summary
In #11770 I made a note that we leave subdocuments in
initstate aftersave()to work around some issues like #6818. That is technically incorrect, becauseinitshould only be used for paths that are loaded from the database. The underlying issue comes down to the fact that array subdocument paths'doValidate()function doesn't run validation on paths in the document.In this PR, I made it so that the embedded schema type in document arrays have a
doValidate()that is the same asSubdocumentPath's. In order to support this change, needed to fix a couple of places where we relied on the old behavior in update validators andModel.validate().Future work: make array subdocument paths a separate class, and make primitive arrays also validate all their subpaths when calling
doValidate().We should probably put this change in 6.4, or hold off until after 6.4 It is a fairly sizable change of Mongoose internals, and while our tests do all pass, I'd like to be cautious with this one.
Examples