From 39b0b1b288769eb575cae34a616fb3cf655edbcf Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Mon, 24 Jul 2023 13:30:35 -0400 Subject: [PATCH 01/10] fix: timestamps:false on bulkWrite works --- lib/helpers/model/castBulkWrite.js | 10 ++++--- test/model.test.js | 45 ++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/lib/helpers/model/castBulkWrite.js b/lib/helpers/model/castBulkWrite.js index 4ae1f47efc8..1b0cafaef53 100644 --- a/lib/helpers/model/castBulkWrite.js +++ b/lib/helpers/model/castBulkWrite.js @@ -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']); @@ -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 || op['updateOne'].timestamps == null) { + applyTimestampsToChildren(now, op['updateOne']['update'], model.schema); + } if (op['updateOne'].setDefaultsOnInsert !== false) { setDefaultsOnInsert(op['updateOne']['filter'], model.schema, op['updateOne']['update'], { @@ -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 || op['updateMany'].timestamps == null) { + applyTimestampsToChildren(now, op['updateMany']['update'], model.schema); + } _addDiscriminatorToObject(schema, op['updateMany']['filter']); diff --git a/test/model.test.js b/test/model.test.js index 0b2303a8ff1..14d4bef73a0 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -5732,6 +5732,51 @@ 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 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({ From 1be8a808d31cd62e6e9c57a12ef38bc38b843502 Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Mon, 24 Jul 2023 14:48:00 -0400 Subject: [PATCH 02/10] made requested changes --- lib/helpers/model/castBulkWrite.js | 4 ++-- test/model.test.js | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/helpers/model/castBulkWrite.js b/lib/helpers/model/castBulkWrite.js index 1b0cafaef53..94a38806029 100644 --- a/lib/helpers/model/castBulkWrite.js +++ b/lib/helpers/model/castBulkWrite.js @@ -65,7 +65,7 @@ module.exports = function castBulkWrite(originalModel, op, options) { applyTimestampsToUpdate(now, createdAt, updatedAt, op['updateOne']['update'], {}); } - if (op['updateOne'].timestamps || op['updateOne'].timestamps == null) { + if (op['updateOne'].timestamps !== false) { applyTimestampsToChildren(now, op['updateOne']['update'], model.schema); } @@ -118,7 +118,7 @@ module.exports = function castBulkWrite(originalModel, op, options) { const updatedAt = model.schema.$timestamps.updatedAt; applyTimestampsToUpdate(now, createdAt, updatedAt, op['updateMany']['update'], {}); } - if (op['updateMany'].timestamps || op['updateMany'].timestamps == null) { + if (op['updateMany'].timestamps !== false) { applyTimestampsToChildren(now, op['updateMany']['update'], model.schema); } diff --git a/test/model.test.js b/test/model.test.js index 14d4bef73a0..be21e61821f 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -5754,6 +5754,7 @@ describe('Model', function() { isDeleted: false }); const initialTime = entry.posts[0].updatedAt; + await delay(5000); await User.bulkWrite([{ updateMany: { @@ -5775,7 +5776,7 @@ describe('Model', function() { const res = await User.findOne({ _id: entry._id }); const currentTime = res.posts[0].updatedAt; assert.equal(initialTime.getTime(), currentTime.getTime()); - }); + }).timeout(7000); it('bulkWrite can overwrite schema `strict` option for filters and updates (gh-8778)', async function() { // Arrange From 26641c008157dd9eb8943c5ed248dc9ff72dce58 Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Mon, 24 Jul 2023 17:42:52 -0400 Subject: [PATCH 03/10] revert timeout settings + fix delay --- test/model.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/model.test.js b/test/model.test.js index be21e61821f..d7dc58c4afb 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -5754,7 +5754,7 @@ describe('Model', function() { isDeleted: false }); const initialTime = entry.posts[0].updatedAt; - await delay(5000); + await delay(500); await User.bulkWrite([{ updateMany: { @@ -5776,7 +5776,7 @@ describe('Model', function() { const res = await User.findOne({ _id: entry._id }); const currentTime = res.posts[0].updatedAt; assert.equal(initialTime.getTime(), currentTime.getTime()); - }).timeout(7000); + }); it('bulkWrite can overwrite schema `strict` option for filters and updates (gh-8778)', async function() { // Arrange From 80e2d08cfd226f2acf576c2570df1cf8ded0bd42 Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Tue, 25 Jul 2023 11:30:08 -0400 Subject: [PATCH 04/10] disable id virtual if `alias: id` set --- lib/helpers/schema/idGetter.js | 3 +++ test/schema.alias.test.js | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/lib/helpers/schema/idGetter.js b/lib/helpers/schema/idGetter.js index ba5ef1011c2..6df8a8cc04f 100644 --- a/lib/helpers/schema/idGetter.js +++ b/lib/helpers/schema/idGetter.js @@ -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); diff --git a/test/schema.alias.test.js b/test/schema.alias.test.js index 0f37f595c1a..9e0733de958 100644 --- a/test/schema.alias.test.js +++ b/test/schema.alias.test.js @@ -183,4 +183,25 @@ 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 removeMultipleSpaces = (val) => { + return val.replace(/\s+/g, ' '); + }; + + const testSchema = new Schema({ + _id: { + type: String, + required: true, + set: removeMultipleSpaces, + alias: 'id' + } + }); + const Test = db.model('gh13650', testSchema); + const doc = new Test({ + id: 'H-1' + }); + await doc.save(); + assert.ok(doc); + }); }); From a6bf1161e26db49dbb5de912f68b70ec203aaae2 Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Tue, 25 Jul 2023 11:30:47 -0400 Subject: [PATCH 05/10] fix:lint --- test/schema.alias.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/schema.alias.test.js b/test/schema.alias.test.js index 9e0733de958..7596f0faa93 100644 --- a/test/schema.alias.test.js +++ b/test/schema.alias.test.js @@ -188,7 +188,7 @@ describe('schema alias option', function() { const removeMultipleSpaces = (val) => { return val.replace(/\s+/g, ' '); }; - + const testSchema = new Schema({ _id: { type: String, From 5db8ce2e3f75afe589b71b3d80fe37e3d3001c95 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 25 Jul 2023 15:40:24 -0400 Subject: [PATCH 06/10] Update test/schema.alias.test.js Co-authored-by: Ostap Skryshevskyi --- test/schema.alias.test.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/schema.alias.test.js b/test/schema.alias.test.js index 7596f0faa93..0e9a96e6a5b 100644 --- a/test/schema.alias.test.js +++ b/test/schema.alias.test.js @@ -185,15 +185,11 @@ describe('schema alias option', function() { }); it('should disable the id virtual entirely if there\'s a field with alias `id` gh-13650', async function() { - const removeMultipleSpaces = (val) => { - return val.replace(/\s+/g, ' '); - }; - const testSchema = new Schema({ _id: { type: String, required: true, - set: removeMultipleSpaces, + set: (val) => val.replace(/\s+/g, ' '), alias: 'id' } }); From 840ab967be90bd61df22239091283a134d434fea Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 25 Jul 2023 15:50:54 -0400 Subject: [PATCH 07/10] Update model.test.js --- test/model.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/model.test.js b/test/model.test.js index d7dc58c4afb..af1f267cf05 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -5754,7 +5754,7 @@ describe('Model', function() { isDeleted: false }); const initialTime = entry.posts[0].updatedAt; - await delay(500); + await delay(10); await User.bulkWrite([{ updateMany: { From 14a6a02adcd05dada2fbb87250ed0737900f5eee Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Wed, 26 Jul 2023 11:29:59 -0400 Subject: [PATCH 08/10] fix: setting `flattenObjectIds` returns null doc --- lib/model.js | 3 ++- test/schema.test.js | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/model.js b/lib/model.js index 595d33178fe..cc0ede41af6 100644 --- a/lib/model.js +++ b/lib/model.js @@ -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 }); /** diff --git a/test/schema.test.js b/test/schema.test.js index 54145e1c0ea..e9ac1bf8cbf 100644 --- a/test/schema.test.js +++ b/test/schema.test.js @@ -3097,4 +3097,14 @@ describe('schema', function() { assert.ok(res[0].tags.createdAt); assert.ok(res[0].tags.updatedAt); }); + it('should not have side effects when enabling 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.findById(doc._id); + assert.equal(res.name, 'Test Testerson'); + }); }); From 93ccb6637c0032da77bca233627ddf9ab71902be Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 26 Jul 2023 12:19:20 -0400 Subject: [PATCH 09/10] Update schema.test.js --- test/schema.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/schema.test.js b/test/schema.test.js index e9ac1bf8cbf..da6055067bf 100644 --- a/test/schema.test.js +++ b/test/schema.test.js @@ -3097,14 +3097,14 @@ describe('schema', function() { assert.ok(res[0].tags.createdAt); assert.ok(res[0].tags.updatedAt); }); - it('should not have side effects when enabling the `flattenObjectIds` option gh-13648', async function() { + 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.findById(doc._id); + const res = await Test.findOne({ _id: { $eq: doc._id, $type: 'objectId' } }); assert.equal(res.name, 'Test Testerson'); }); }); From 6bf54900dc9f89960d38cb8803a7fa3561fcfec6 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 31 Jul 2023 13:48:57 -0400 Subject: [PATCH 10/10] fix(query): remove unnecessary check for atomic operators in findOneAndReplace() --- lib/query.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/query.js b/lib/query.js index 44823cd0573..d64bb03f34d 100644 --- a/lib/query.js +++ b/lib/query.js @@ -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); }