Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(array): avoid converting to $set when calling pull() on an element in the middle of the array #14531

Merged
merged 3 commits into from
Apr 27, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 17 additions & 3 deletions lib/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -2381,15 +2381,29 @@ Document.prototype.isDirectModified = function(path) {
}

if (typeof path === 'string' && path.indexOf(' ') === -1) {
return this.$__.activePaths.getStatePaths('modify').hasOwnProperty(path);
const res = this.$__.activePaths.getStatePaths('modify').hasOwnProperty(path);
if (res || path.indexOf('.') === -1) {
return res;
}

const pieces = path.split('.');
for (let i = 0; i < pieces.length - 1; ++i) {
const subpath = pieces.slice(0, i + 1).join('.');
const subdoc = this.$get(subpath);
if (subdoc != null && subdoc.$__ != null && subdoc.isDirectModified(pieces.slice(i + 1).join('.'))) {
return true;
}
}

return false;
}

let paths = path;
if (!Array.isArray(paths)) {
if (typeof paths === 'string') {
paths = paths.split(' ');
}

return paths.some(path => this.$__.activePaths.getStatePaths('modify').hasOwnProperty(path));
return paths.some(path => this.isDirectModified(path));
};

/**
Expand Down
11 changes: 7 additions & 4 deletions lib/types/array/methods/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,10 @@ const methods = {

pull() {
const values = [].map.call(arguments, (v, i) => this._cast(v, i, { defaults: false }), this);
const cur = this[arrayParentSymbol].get(this[arrayPathSymbol]);
let cur = this[arrayParentSymbol].get(this[arrayPathSymbol]);
if (utils.isMongooseArray(cur)) {
cur = cur.__array;
}
let i = cur.length;
let mem;
this._markModified();
Expand All @@ -615,10 +618,10 @@ const methods = {
return mem.equals(v);
});
if (some) {
[].splice.call(cur, i, 1);
cur.splice(i, 1);
}
} else if (~cur.indexOf.call(values, mem)) {
[].splice.call(cur, i, 1);
} else if (~this.indexOf.call(values, mem)) {
cur.splice(i, 1);
}
}

Expand Down
58 changes: 58 additions & 0 deletions test/document.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13055,6 +13055,64 @@ describe('document', function() {
const obj = parent.toJSON({ getters: true });
assert.equal(obj.children[0].name, 'Stefan');
});

it('isDirectModified on paths underneath direct modified subdoc (gh-14502)', async function() {
const JsonFieldSchema = new Schema({
fieldA: String,
fieldB: String
});

const CommentSchema = new Schema({
title: String,
body: String,
jsonField: JsonFieldSchema
});

const blogPostSchema = new Schema({
comment: CommentSchema
});

const Comments = db.model('Comments', CommentSchema);
const BlogPost = db.model('BlogPost', blogPostSchema);
vkarpov15 marked this conversation as resolved.
Show resolved Hide resolved

const comment1 = new Comments({});
comment1.init({
title: 'Test',
body: 'Test',
jsonField: {
fieldA: 'field A',
fieldB: 'field B'
}
});

const update = {
title: 'New test',
jsonField: {
fieldA: 'new Field A'
}
};
Object.assign(comment1, { ...update });

assert.ok(comment1.isDirectModified('jsonField.fieldA'));
assert.ok(comment1.jsonField.isDirectModified('fieldA'));

const blogPost = new BlogPost({});
blogPost.init({
comment: {
title: 'Test',
body: 'Test',
jsonField: {
fieldA: 'field A',
fieldB: 'field B'
}
}
});

Object.assign(blogPost.comment, { ...update });

assert.ok(blogPost.isDirectModified('comment.jsonField.fieldA'));
assert.ok(blogPost.comment.jsonField.isDirectModified('fieldA'));
});
});

describe('Check if instance function that is supplied in schema option is availabe', function() {
Expand Down
21 changes: 21 additions & 0 deletions test/types.array.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,27 @@ describe('types array', function() {
doc.a.pull(cat.id);
assert.equal(doc.a.length, 0);

assert.ok(doc.getChanges().$pullAll.a);
});

it('registers $pull atomic if pulling from middle (gh-14502)', async function() {
const schema = new Schema({
a: [{ type: Schema.ObjectId, ref: 'Cat' }]
});
const A = db.model('Test', schema);

const oid1 = new mongoose.Types.ObjectId();
const oid2 = new mongoose.Types.ObjectId();
const oid3 = new mongoose.Types.ObjectId();
const a = new A({ a: [oid1, oid2, oid3] });
await a.save();

const doc = await A.findById(a);
assert.equal(doc.a.length, 3);
doc.a.pull(oid2);
assert.equal(doc.a.length, 2);

assert.ok(doc.getChanges().$pullAll.a);
});

it('handles pulling with no _id (gh-3341)', async function() {
Expand Down