Skip to content

Commit e94ca23

Browse files
vkarpov15vkarpov15
authored
Merge pull request #13589 from Automattic/vkarpov15/gh-13582
fix(document): clean up all array subdocument modified paths on save()
2 parents 1a998e2 + 422dff4 commit e94ca23

File tree

5 files changed

+64
-36
lines changed

5 files changed

+64
-36
lines changed

‎lib/document.js‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2684,7 +2684,7 @@ function _getPathsToValidate(doc, pathsToValidate, pathsToSkip) {
26842684

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

27722772
for (const path of paths) {
27732773
const _pathType = doc.$__schema.path(path);
2774-
if (!_pathType || !_pathType.$isSchemaMap) {
2774+
2775+
if (!_pathType) {
2776+
continue;
2777+
}
2778+
2779+
// If underneath a document array, may need to re-validate the parent
2780+
// array re: gh-6818. Do this _after_ adding subpaths, because
2781+
// we don't want to add every array subpath.
2782+
if (_pathType.$parentSchemaDocArray && typeof _pathType.$parentSchemaDocArray.path === 'string') {
2783+
paths.add(_pathType.$parentSchemaDocArray.path);
2784+
}
2785+
2786+
if (!_pathType.$isSchemaMap) {
27752787
continue;
27762788
}
27772789

@@ -3318,14 +3330,7 @@ Document.prototype.$__reset = function reset() {
33183330
if (this.isModified(fullPathWithIndexes) || isParentInit(fullPathWithIndexes)) {
33193331
subdoc.$__reset();
33203332
if (subdoc.$isDocumentArrayElement) {
3321-
if (!resetArrays.has(subdoc.parentArray())) {
3322-
const array = subdoc.parentArray();
3323-
this.$__.activePaths.clearPath(fullPathWithIndexes.replace(/\.\d+$/, '').slice(-subdoc.$basePath - 1));
3324-
array[arrayAtomicsBackupSymbol] = array[arrayAtomicsSymbol];
3325-
array[arrayAtomicsSymbol] = {};
3326-
3327-
resetArrays.add(array);
3328-
}
3333+
resetArrays.add(subdoc.parentArray());
33293334
} else {
33303335
if (subdoc.$parent() === this) {
33313336
this.$__.activePaths.clearPath(subdoc.$basePath);
@@ -3338,6 +3343,12 @@ Document.prototype.$__reset = function reset() {
33383343
}
33393344
}
33403345

3346+
for (const array of resetArrays) {
3347+
this.$__.activePaths.clearPath(array.$path());
3348+
array[arrayAtomicsBackupSymbol] = array[arrayAtomicsSymbol];
3349+
array[arrayAtomicsSymbol] = {};
3350+
}
3351+
33413352
function isParentInit(path) {
33423353
path = path.indexOf('.') === -1 ? [path] : path.split('.');
33433354
let cur = '';

‎lib/helpers/populate/getModelsMapForPopulate.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ module.exports = function getModelsMapForPopulate(model, docs, options) {
6464
schema.options.refPath == null) {
6565
continue;
6666
}
67-
const isUnderneathDocArray = schema && schema.$isUnderneathDocArray;
67+
const isUnderneathDocArray = schema && schema.$parentSchemaDocArray;
6868
if (isUnderneathDocArray && get(options, 'options.sort') != null) {
6969
return new MongooseError('Cannot populate with `sort` on path ' + options.path +
7070
' because it is a subproperty of a document array');

‎lib/helpers/populate/getSchemaTypes.js‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,8 @@ module.exports = function getSchemaTypes(model, schema, doc, path) {
101101
nestedPath.concat(parts.slice(0, p))
102102
);
103103
if (ret) {
104-
ret.$isUnderneathDocArray = ret.$isUnderneathDocArray ||
105-
!foundschema.schema.$isSingleNested;
104+
ret.$parentSchemaDocArray = ret.$parentSchemaDocArray ||
105+
(foundschema.schema.$isSingleNested ? null : foundschema);
106106
}
107107
return ret;
108108
}
@@ -117,10 +117,10 @@ module.exports = function getSchemaTypes(model, schema, doc, path) {
117117
nestedPath.concat(parts.slice(0, p))
118118
);
119119
if (_ret != null) {
120-
_ret.$isUnderneathDocArray = _ret.$isUnderneathDocArray ||
121-
!foundschema.schema.$isSingleNested;
122-
if (_ret.$isUnderneathDocArray) {
123-
ret.$isUnderneathDocArray = true;
120+
_ret.$parentSchemaDocArray = _ret.$parentSchemaDocArray ||
121+
(foundschema.schema.$isSingleNested ? null : foundschema);
122+
if (_ret.$parentSchemaDocArray) {
123+
ret.$parentSchemaDocArray = _ret.$parentSchemaDocArray;
124124
}
125125
ret.push(_ret);
126126
}
@@ -135,8 +135,8 @@ module.exports = function getSchemaTypes(model, schema, doc, path) {
135135
);
136136

137137
if (ret) {
138-
ret.$isUnderneathDocArray = ret.$isUnderneathDocArray ||
139-
!foundschema.schema.$isSingleNested;
138+
ret.$parentSchemaDocArray = ret.$parentSchemaDocArray ||
139+
(foundschema.schema.$isSingleNested ? null : foundschema);
140140
}
141141
return ret;
142142
}
@@ -188,10 +188,6 @@ module.exports = function getSchemaTypes(model, schema, doc, path) {
188188
nestedPath.concat(parts.slice(0, p))
189189
);
190190

191-
if (ret) {
192-
ret.$isUnderneathDocArray = ret.$isUnderneathDocArray ||
193-
!model.schema.$isSingleNested;
194-
}
195191
return ret;
196192
}
197193
}
@@ -212,8 +208,8 @@ module.exports = function getSchemaTypes(model, schema, doc, path) {
212208
);
213209

214210
if (ret != null) {
215-
ret.$isUnderneathDocArray = ret.$isUnderneathDocArray ||
216-
!schema.$isSingleNested;
211+
ret.$parentSchemaDocArray = ret.$parentSchemaDocArray ||
212+
(schema.$isSingleNested ? null : schema);
217213
return ret;
218214
}
219215
}

‎lib/schema.js‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,22 +1131,22 @@ Schema.prototype.path = function(path, obj) {
11311131
for (const key of Object.keys(schemaType.schema.paths)) {
11321132
const _schemaType = schemaType.schema.paths[key];
11331133
this.subpaths[path + '.' + key] = _schemaType;
1134-
if (typeof _schemaType === 'object' && _schemaType != null) {
1135-
_schemaType.$isUnderneathDocArray = true;
1134+
if (typeof _schemaType === 'object' && _schemaType != null && _schemaType.$parentSchemaDocArray == null) {
1135+
_schemaType.$parentSchemaDocArray = schemaType;
11361136
}
11371137
}
11381138
for (const key of Object.keys(schemaType.schema.subpaths)) {
11391139
const _schemaType = schemaType.schema.subpaths[key];
11401140
this.subpaths[path + '.' + key] = _schemaType;
1141-
if (typeof _schemaType === 'object' && _schemaType != null) {
1142-
_schemaType.$isUnderneathDocArray = true;
1141+
if (typeof _schemaType === 'object' && _schemaType != null && _schemaType.$parentSchemaDocArray == null) {
1142+
_schemaType.$parentSchemaDocArray = schemaType;
11431143
}
11441144
}
11451145
for (const key of Object.keys(schemaType.schema.singleNestedPaths)) {
11461146
const _schemaType = schemaType.schema.singleNestedPaths[key];
11471147
this.subpaths[path + '.' + key] = _schemaType;
1148-
if (typeof _schemaType === 'object' && _schemaType != null) {
1149-
_schemaType.$isUnderneathDocArray = true;
1148+
if (typeof _schemaType === 'object' && _schemaType != null && _schemaType.$parentSchemaDocArray == null) {
1149+
_schemaType.$parentSchemaDocArray = schemaType;
11501150
}
11511151
}
11521152
}
@@ -2556,16 +2556,16 @@ Schema.prototype._getSchema = function(path) {
25562556
// comments.$.comments.$.title
25572557
ret = search(parts.slice(p + 1), foundschema.schema);
25582558
if (ret) {
2559-
ret.$isUnderneathDocArray = ret.$isUnderneathDocArray ||
2560-
!foundschema.schema.$isSingleNested;
2559+
ret.$parentSchemaDocArray = ret.$parentSchemaDocArray ||
2560+
(foundschema.schema.$isSingleNested ? null : foundschema);
25612561
}
25622562
return ret;
25632563
}
25642564
// this is the last path of the selector
25652565
ret = search(parts.slice(p), foundschema.schema);
25662566
if (ret) {
2567-
ret.$isUnderneathDocArray = ret.$isUnderneathDocArray ||
2568-
!foundschema.schema.$isSingleNested;
2567+
ret.$parentSchemaDocArray = ret.$parentSchemaDocArray ||
2568+
(foundschema.schema.$isSingleNested ? null : foundschema);
25692569
}
25702570
return ret;
25712571
}

‎test/document.test.js‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6285,7 +6285,9 @@ describe('document', function() {
62856285
name: String,
62866286
folders: {
62876287
type: [{ folderId: String }],
6288-
validate: v => assert.ok(v.length === new Set(v.map(el => el.folderId)).size, 'Duplicate')
6288+
validate: v => {
6289+
assert.ok(v.length === new Set(v.map(el => el.folderId)).size, 'Duplicate');
6290+
}
62896291
}
62906292
}]
62916293
}
@@ -12212,6 +12214,25 @@ describe('document', function() {
1221212214
const fromDb = await Test.findById(x._id).lean();
1221312215
assert.equal(fromDb.c.x.y, 1);
1221412216
});
12217+
12218+
it('cleans up all array subdocs modified state on save (gh-13582)', async function() {
12219+
const ElementSchema = new mongoose.Schema({
12220+
elementName: String
12221+
});
12222+
12223+
const MyDocSchema = new mongoose.Schema({
12224+
docName: String,
12225+
elements: [ElementSchema]
12226+
});
12227+
12228+
const Test = db.model('Test', MyDocSchema);
12229+
let doc = new Test({ docName: 'MyDocName' });
12230+
doc.elements.push({ elementName: 'ElementName1' });
12231+
doc.elements.push({ elementName: 'ElementName2' });
12232+
doc = await doc.save();
12233+
assert.deepStrictEqual(doc.elements[0].modifiedPaths(), []);
12234+
assert.deepStrictEqual(doc.elements[1].modifiedPaths(), []);
12235+
});
1221512236
});
1221612237

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

0 commit comments

Comments
 (0)