Conversation
vkarpov15
requested review from
AbdelrahmanHafez and
hasezoey
and
a lite review from Copilot
August 30, 2026 20:50
vkarpov15
marked this pull request as draft
September 4, 2026 17:58
Collaborator
Author
|
Converting this to a draft because there's a lot of details and nuance to investigate here. |
…sets and n^2 loops
Comment on lines
+3285
to
+3289
| const parentDocArray = type.$parentSchemaDocArray; | ||
| if (parentDocArray != null) { | ||
| const arrayPath = parentDocArray.path; | ||
| if (typeof arrayPath === 'string' && schemaPaths[arrayPath] === parentDocArray && | ||
| covered[arrayPath] !== true) { |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ic/mongoose into vkarpov15/paths-to-validate-fix
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
While working on #16291 I spotted an inconsistency in how
validate()andvalidateSync()work on document arrays underneath nested paths. In master, switching the test from #5411 to usevalidate()instead ofvalidateSync()fails:validate()still double-runs validators on nested document arrays,validateSync()does not.I fixed that by avoiding adding the array path in
_getPathsToValidate()in the nested path case and adding a consistentdoValidateSync()override forSchemaDocumentArrayElementanalogous to the override fordoValidate().That also turned up some related issues when testing
validate()in parallel withvalidateSync()in some related tests that errored while I was working on the original problem related to skipSchemaValidators: with the new behavior, document arrays no longer needskipSchemaValidatorsto avoid double-calling schema validators. Still necessary for subdocuments though.Further work in b91495f and 4443584 ended up as a full rewrite of
_getPathsToValidate()focusing on simplification and performance improvements. Existing_getPathsToValidate()added unnecessary paths and then cleaned them up with an o(n^2) loop that always bugged me. It turns out this loop was only necessary for two cases that we can handle separately:arr.0(handled by the newcoveredcheck) andarr.0.subprop(handled by_isValidatedBySubdoc). The new_getPathsToValidate()looks to be significantly faster as well as simpler.Examples