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(query): remove unnecessary check for atomic operators in findOneAndReplace() #13678

Merged
merged 13 commits into from Aug 1, 2023
Merged
10 changes: 6 additions & 4 deletions lib/helpers/model/castBulkWrite.js
Expand Up @@ -20,7 +20,6 @@ const setDefaultsOnInsert = require('../setDefaultsOnInsert');

module.exports = function castBulkWrite(originalModel, op, options) {
const now = originalModel.base.now();

if (op['insertOne']) {
return (callback) => {
const model = decideModelByObject(originalModel, op['insertOne']['document']);
Expand Down Expand Up @@ -66,7 +65,9 @@ module.exports = function castBulkWrite(originalModel, op, options) {
applyTimestampsToUpdate(now, createdAt, updatedAt, op['updateOne']['update'], {});
}

applyTimestampsToChildren(now, op['updateOne']['update'], model.schema);
if (op['updateOne'].timestamps !== false) {
applyTimestampsToChildren(now, op['updateOne']['update'], model.schema);
}

if (op['updateOne'].setDefaultsOnInsert !== false) {
setDefaultsOnInsert(op['updateOne']['filter'], model.schema, op['updateOne']['update'], {
Expand Down Expand Up @@ -117,8 +118,9 @@ module.exports = function castBulkWrite(originalModel, op, options) {
const updatedAt = model.schema.$timestamps.updatedAt;
applyTimestampsToUpdate(now, createdAt, updatedAt, op['updateMany']['update'], {});
}

applyTimestampsToChildren(now, op['updateMany']['update'], model.schema);
if (op['updateMany'].timestamps !== false) {
applyTimestampsToChildren(now, op['updateMany']['update'], model.schema);
}

_addDiscriminatorToObject(schema, op['updateMany']['filter']);

Expand Down
3 changes: 3 additions & 0 deletions lib/helpers/schema/idGetter.js
Expand Up @@ -12,6 +12,9 @@ module.exports = function addIdGetter(schema) {
if (!autoIdGetter) {
return schema;
}
if (schema.aliases && schema.aliases.id) {
return schema;
}
schema.virtual('id').get(idGetter);
schema.virtual('id').set(idSetter);

Expand Down
3 changes: 2 additions & 1 deletion lib/model.js
Expand Up @@ -77,7 +77,8 @@ const modelSymbol = require('./helpers/symbols').modelSymbol;
const subclassedSymbol = Symbol('mongoose#Model#subclassed');

const saveToObjectOptions = Object.assign({}, internalToObjectOptions, {
bson: true
bson: true,
flattenObjectIds: false
});

/**
Expand Down
3 changes: 0 additions & 3 deletions lib/query.js
Expand Up @@ -3587,9 +3587,6 @@ Query.prototype.findOneAndReplace = function(filter, replacement, options) {
}

if (replacement != null) {
if (hasDollarKeys(replacement)) {
throw new Error('The replacement document must not contain atomic operators.');
}
this._mergeUpdate(replacement);
}

Expand Down
46 changes: 46 additions & 0 deletions test/model.test.js
Expand Up @@ -5732,6 +5732,52 @@ describe('Model', function() {

});

it('bulkwrite should not change updatedAt on subdocs when timestamps set to false (gh-13611)', async function() {

const postSchema = new Schema({
title: String,
category: String,
isDeleted: Boolean
}, { timestamps: true });

const userSchema = new Schema({
name: String,
isDeleted: Boolean,
posts: { type: [postSchema] }
}, { timestamps: true });

const User = db.model('gh13611User', userSchema);

const entry = await User.create({
name: 'Test Testerson',
posts: [{ title: 'title a', category: 'a', isDeleted: false }, { title: 'title b', category: 'b', isDeleted: false }],
isDeleted: false
});
const initialTime = entry.posts[0].updatedAt;
await delay(10);

await User.bulkWrite([{
updateMany: {
filter: {
isDeleted: false
},
update: {
'posts.$[post].isDeleted': true
},
arrayFilters: [
{
'post.category': { $eq: 'a' }
}
],
upsert: false,
timestamps: false
}
}]);
const res = await User.findOne({ _id: entry._id });
const currentTime = res.posts[0].updatedAt;
assert.equal(initialTime.getTime(), currentTime.getTime());
});

it('bulkWrite can overwrite schema `strict` option for filters and updates (gh-8778)', async function() {
// Arrange
const userSchema = new Schema({
Expand Down
17 changes: 17 additions & 0 deletions test/schema.alias.test.js
Expand Up @@ -183,4 +183,21 @@ describe('schema alias option', function() {
assert.ok(schema.virtuals['name1']);
assert.ok(schema.virtuals['name2']);
});
it('should disable the id virtual entirely if there\'s a field with alias `id` gh-13650', async function() {

const testSchema = new Schema({
_id: {
type: String,
required: true,
set: (val) => val.replace(/\s+/g, ' '),
alias: 'id'
}
});
const Test = db.model('gh13650', testSchema);
const doc = new Test({
id: 'H-1'
});
await doc.save();
assert.ok(doc);
});
});
10 changes: 10 additions & 0 deletions test/schema.test.js
Expand Up @@ -3097,4 +3097,14 @@ describe('schema', function() {
assert.ok(res[0].tags.createdAt);
assert.ok(res[0].tags.updatedAt);
});
it('should not save objectids as strings when using the `flattenObjectIds` option (gh-13648)', async function() {
const testSchema = new Schema({
name: String
}, { toObject: { flattenObjectIds: true } });
const Test = db.model('gh13648', testSchema);

const doc = await Test.create({ name: 'Test Testerson' });
const res = await Test.findOne({ _id: { $eq: doc._id, $type: 'objectId' } });
assert.equal(res.name, 'Test Testerson');
});
});