Skip to content

Commit

Permalink
Merge pull request #13589 from Automattic/vkarpov15/gh-13582
Browse files Browse the repository at this point in the history 
fix(document): clean up all array subdocument modified paths on save()
  • Loading branch information
@vkarpov15
vkarpov15 authored Jul 10, 2023
2 parents 1a998e2 + 422dff4 commit e94ca23
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 36 deletions.
31 changes: 21 additions & 10 deletions lib/document.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2684,7 +2684,7 @@ function _getPathsToValidate(doc, pathsToValidate, pathsToSkip) {

// Optimization: if primitive path with no validators, or array of primitives
// with no validators, skip validating this path entirely.
if (!_pathType.caster && _pathType.validators.length === 0) {
if (!_pathType.caster && _pathType.validators.length === 0 && !_pathType.$parentSchemaDocArray) {
paths.delete(path);
} else if (_pathType.$isMongooseArray &&
!_pathType.$isMongooseDocumentArray && // Skip document arrays...
Expand Down Expand Up @@ -2771,7 +2771,19 @@ function _getPathsToValidate(doc, pathsToValidate, pathsToSkip) {

for (const path of paths) {
const _pathType = doc.$__schema.path(path);
if (!_pathType || !_pathType.$isSchemaMap) {

if (!_pathType) {
continue;
}

// If underneath a document array, may need to re-validate the parent
// array re: gh-6818. Do this _after_ adding subpaths, because
// we don't want to add every array subpath.
if (_pathType.$parentSchemaDocArray && typeof _pathType.$parentSchemaDocArray.path === 'string') {
paths.add(_pathType.$parentSchemaDocArray.path);
}

if (!_pathType.$isSchemaMap) {
continue;
}

Expand Down Expand Up @@ -3318,14 +3330,7 @@ Document.prototype.$__reset = function reset() {
if (this.isModified(fullPathWithIndexes) || isParentInit(fullPathWithIndexes)) {
subdoc.$__reset();
if (subdoc.$isDocumentArrayElement) {
if (!resetArrays.has(subdoc.parentArray())) {
const array = subdoc.parentArray();
this.$__.activePaths.clearPath(fullPathWithIndexes.replace(/\.\d+$/, '').slice(-subdoc.$basePath - 1));
array[arrayAtomicsBackupSymbol] = array[arrayAtomicsSymbol];
array[arrayAtomicsSymbol] = {};

resetArrays.add(array);
}
resetArrays.add(subdoc.parentArray());
} else {
if (subdoc.$parent() === this) {
this.$__.activePaths.clearPath(subdoc.$basePath);
Expand All @@ -3338,6 +3343,12 @@ Document.prototype.$__reset = function reset() {
}
}

for (const array of resetArrays) {
this.$__.activePaths.clearPath(array.$path());
array[arrayAtomicsBackupSymbol] = array[arrayAtomicsSymbol];
array[arrayAtomicsSymbol] = {};
}

function isParentInit(path) {
path = path.indexOf('.') === -1 ? [path] : path.split('.');
let cur = '';
Expand Down
2 changes: 1 addition & 1 deletion lib/helpers/populate/getModelsMapForPopulate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ module.exports = function getModelsMapForPopulate(model, docs, options) {
schema.options.refPath == null) {
continue;
}
const isUnderneathDocArray = schema && schema.$isUnderneathDocArray;
const isUnderneathDocArray = schema && schema.$parentSchemaDocArray;
if (isUnderneathDocArray && get(options, 'options.sort') != null) {
return new MongooseError('Cannot populate with `sort` on path ' + options.path +
' because it is a subproperty of a document array');
Expand Down
24 changes: 10 additions & 14 deletions lib/helpers/populate/getSchemaTypes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ module.exports = function getSchemaTypes(model, schema, doc, path) {
nestedPath.concat(parts.slice(0, p))
);
if (ret) {
ret.$isUnderneathDocArray = ret.$isUnderneathDocArray ||
!foundschema.schema.$isSingleNested;
ret.$parentSchemaDocArray = ret.$parentSchemaDocArray ||
(foundschema.schema.$isSingleNested ? null : foundschema);
}
return ret;
}
Expand All @@ -117,10 +117,10 @@ module.exports = function getSchemaTypes(model, schema, doc, path) {
nestedPath.concat(parts.slice(0, p))
);
if (_ret != null) {
_ret.$isUnderneathDocArray = _ret.$isUnderneathDocArray ||
!foundschema.schema.$isSingleNested;
if (_ret.$isUnderneathDocArray) {
ret.$isUnderneathDocArray = true;
_ret.$parentSchemaDocArray = _ret.$parentSchemaDocArray ||
(foundschema.schema.$isSingleNested ? null : foundschema);
if (_ret.$parentSchemaDocArray) {
ret.$parentSchemaDocArray = _ret.$parentSchemaDocArray;
}
ret.push(_ret);
}
Expand All @@ -135,8 +135,8 @@ module.exports = function getSchemaTypes(model, schema, doc, path) {
);

if (ret) {
ret.$isUnderneathDocArray = ret.$isUnderneathDocArray ||
!foundschema.schema.$isSingleNested;
ret.$parentSchemaDocArray = ret.$parentSchemaDocArray ||
(foundschema.schema.$isSingleNested ? null : foundschema);
}
return ret;
}
Expand Down Expand Up @@ -188,10 +188,6 @@ module.exports = function getSchemaTypes(model, schema, doc, path) {
nestedPath.concat(parts.slice(0, p))
);

if (ret) {
ret.$isUnderneathDocArray = ret.$isUnderneathDocArray ||
!model.schema.$isSingleNested;
}
return ret;
}
}
Expand All @@ -212,8 +208,8 @@ module.exports = function getSchemaTypes(model, schema, doc, path) {
);

if (ret != null) {
ret.$isUnderneathDocArray = ret.$isUnderneathDocArray ||
!schema.$isSingleNested;
ret.$parentSchemaDocArray = ret.$parentSchemaDocArray ||
(schema.$isSingleNested ? null : schema);
return ret;
}
}
Expand Down
20 changes: 10 additions & 10 deletions lib/schema.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1131,22 +1131,22 @@ Schema.prototype.path = function(path, obj) {
for (const key of Object.keys(schemaType.schema.paths)) {
const _schemaType = schemaType.schema.paths[key];
this.subpaths[path + '.' + key] = _schemaType;
if (typeof _schemaType === 'object' && _schemaType != null) {
_schemaType.$isUnderneathDocArray = true;
if (typeof _schemaType === 'object' && _schemaType != null && _schemaType.$parentSchemaDocArray == null) {
_schemaType.$parentSchemaDocArray = schemaType;
}
}
for (const key of Object.keys(schemaType.schema.subpaths)) {
const _schemaType = schemaType.schema.subpaths[key];
this.subpaths[path + '.' + key] = _schemaType;
if (typeof _schemaType === 'object' && _schemaType != null) {
_schemaType.$isUnderneathDocArray = true;
if (typeof _schemaType === 'object' && _schemaType != null && _schemaType.$parentSchemaDocArray == null) {
_schemaType.$parentSchemaDocArray = schemaType;
}
}
for (const key of Object.keys(schemaType.schema.singleNestedPaths)) {
const _schemaType = schemaType.schema.singleNestedPaths[key];
this.subpaths[path + '.' + key] = _schemaType;
if (typeof _schemaType === 'object' && _schemaType != null) {
_schemaType.$isUnderneathDocArray = true;
if (typeof _schemaType === 'object' && _schemaType != null && _schemaType.$parentSchemaDocArray == null) {
_schemaType.$parentSchemaDocArray = schemaType;
}
}
}
Expand Down Expand Up @@ -2556,16 +2556,16 @@ Schema.prototype._getSchema = function(path) {
// comments.$.comments.$.title
ret = search(parts.slice(p + 1), foundschema.schema);
if (ret) {
ret.$isUnderneathDocArray = ret.$isUnderneathDocArray ||
!foundschema.schema.$isSingleNested;
ret.$parentSchemaDocArray = ret.$parentSchemaDocArray ||
(foundschema.schema.$isSingleNested ? null : foundschema);
}
return ret;
}
// this is the last path of the selector
ret = search(parts.slice(p), foundschema.schema);
if (ret) {
ret.$isUnderneathDocArray = ret.$isUnderneathDocArray ||
!foundschema.schema.$isSingleNested;
ret.$parentSchemaDocArray = ret.$parentSchemaDocArray ||
(foundschema.schema.$isSingleNested ? null : foundschema);
}
return ret;
}
Expand Down
23 changes: 22 additions & 1 deletion test/document.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6285,7 +6285,9 @@ describe('document', function() {
name: String,
folders: {
type: [{ folderId: String }],
validate: v => assert.ok(v.length === new Set(v.map(el => el.folderId)).size, 'Duplicate')
validate: v => {
assert.ok(v.length === new Set(v.map(el => el.folderId)).size, 'Duplicate');
}
}
}]
}
Expand Down Expand Up @@ -12212,6 +12214,25 @@ describe('document', function() {
const fromDb = await Test.findById(x._id).lean();
assert.equal(fromDb.c.x.y, 1);
});

it('cleans up all array subdocs modified state on save (gh-13582)', async function() {
const ElementSchema = new mongoose.Schema({
elementName: String
});

const MyDocSchema = new mongoose.Schema({
docName: String,
elements: [ElementSchema]
});

const Test = db.model('Test', MyDocSchema);
let doc = new Test({ docName: 'MyDocName' });
doc.elements.push({ elementName: 'ElementName1' });
doc.elements.push({ elementName: 'ElementName2' });
doc = await doc.save();
assert.deepStrictEqual(doc.elements[0].modifiedPaths(), []);
assert.deepStrictEqual(doc.elements[1].modifiedPaths(), []);
});
});

describe('Check if instance function that is supplied in schema option is availabe', function() {
Expand Down

0 comments on commit e94ca23

Please sign in to comment.