From 758eca3a7763ec8434f74ec19d121c975b34612b Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 24 Jun 2024 10:13:06 -0400 Subject: [PATCH 1/7] fix(document): avoid passing validateModifiedOnly to subdocs so subdocs get fully validating if they're directly modified Fix #14677 --- lib/document.js | 1 - test/document.test.js | 82 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/lib/document.js b/lib/document.js index d907886ece3..ffaa066ef58 100644 --- a/lib/document.js +++ b/lib/document.js @@ -3053,7 +3053,6 @@ Document.prototype.$__validate = function(pathsToValidate, options, callback) { const doValidateOptions = { ...doValidateOptionsByPath[path], path: path, - validateModifiedOnly: shouldValidateModifiedOnly, validateAllPaths }; diff --git a/test/document.test.js b/test/document.test.js index e5dcfa3bb1a..3ca3d5c96f3 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -2556,6 +2556,88 @@ describe('document', function() { // Does not throw await Model.create({ name: 'test' }); }); + + it('fully validates modified subdocs (gh-14677)', async function() { + const embedSchema = new mongoose.Schema({ + field1: { + type: String, + required: true + }, + field2: String + }); + const testSchema = new mongoose.Schema({ + testField: { + type: String, + required: true + }, + testArray: [embedSchema], + }); + const TestModel = db.model('Test', testSchema); + + let doc = new TestModel({ testArray: [{ field2: 'test' }] }); + let err = await doc.validate({ validateModifiedOnly: true }).then(() => null, err => err); + assert.ok(err); + assert.ok(err.errors['testArray.0.field1']); + assert.equal(err.errors['testArray.0.field1'].kind, 'required'); + + await TestModel.collection.insertOne(doc.toObject()); + doc = await TestModel.findById(doc._id).orFail(); + doc.testArray[0].field2 = 'test modified'; + err = await doc.validate({ validateModifiedOnly: true }).then(() => null, err => err); + assert.ifError(err); + + err = await doc.validate().then(() => null, err => err); + assert.ok(err); + assert.ok(err.errors['testArray.0.field1']); + assert.equal(err.errors['testArray.0.field1'].kind, 'required'); + + doc.testArray[0] = { field2: 'test modified 3' }; + err = await doc.validate({ validateModifiedOnly: true }).then(() => null, err => err); + assert.ok(err); + assert.ok(err.errors['testArray.0.field1']); + assert.equal(err.errors['testArray.0.field1'].kind, 'required'); + }); + + it('fully validates modified single nested subdocs (gh-14677)', async function() { + const embedSchema = new mongoose.Schema({ + field1: { + type: String, + required: true + }, + field2: String + }); + const testSchema = new mongoose.Schema({ + testField: { + type: String, + required: true + }, + subdoc: embedSchema + }); + const TestModel = db.model('Test', testSchema); + + let doc = new TestModel({ subdoc: { field2: 'test' } }); + let err = await doc.validate({ validateModifiedOnly: true }).then(() => null, err => err); + assert.ok(err); + assert.ok(err.errors['subdoc.field1']); + assert.equal(err.errors['subdoc.field1'].kind, 'required'); + + await TestModel.collection.insertOne(doc.toObject()); + doc = await TestModel.findById(doc._id).orFail(); + doc.subdoc.field2 = 'test modified'; + err = await doc.validate({ validateModifiedOnly: true }).then(() => null, err => err); + assert.ifError(err); + + err = await doc.validate().then(() => null, err => err); + assert.ok(err); + assert.ok(err.errors['subdoc.field1']); + assert.equal(err.errors['subdoc.field1'].kind, 'required'); + + doc.subdoc = { field2: 'test modified 3' }; + err = await doc.validate({ validateModifiedOnly: true }).then(() => null, err => err); + assert.ok(err); + assert.ok(err.errors['subdoc.field1']); + assert.equal(err.errors['subdoc.field1'].kind, 'required'); + }); }); describe('bug fixes', function() { From 36ea38302d0c502a41371e484dc67f2a229f4fa4 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 24 Jun 2024 12:40:41 -0400 Subject: [PATCH 2/7] fix(projection): handle projections on arrays in `Model.hydrate()` projection option Fix #14680 --- lib/helpers/projection/applyProjection.js | 6 ++++++ test/helpers/projection.applyProjection.test.js | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/lib/helpers/projection/applyProjection.js b/lib/helpers/projection/applyProjection.js index 1552e07e686..7a35b128b24 100644 --- a/lib/helpers/projection/applyProjection.js +++ b/lib/helpers/projection/applyProjection.js @@ -35,6 +35,9 @@ function applyExclusiveProjection(doc, projection, hasIncludedChildren, projecti if (doc == null || typeof doc !== 'object') { return doc; } + if (Array.isArray(doc)) { + return doc.map(el => applyExclusiveProjection(el, projection, hasIncludedChildren, projectionLimb, prefix)); + } const ret = { ...doc }; projectionLimb = prefix ? (projectionLimb || {}) : projection; @@ -57,6 +60,9 @@ function applyInclusiveProjection(doc, projection, hasIncludedChildren, projecti if (doc == null || typeof doc !== 'object') { return doc; } + if (Array.isArray(doc)) { + return doc.map(el => applyInclusiveProjection(el, projection, hasIncludedChildren, projectionLimb, prefix)); + } const ret = { ...doc }; projectionLimb = prefix ? (projectionLimb || {}) : projection; diff --git a/test/helpers/projection.applyProjection.test.js b/test/helpers/projection.applyProjection.test.js index fadfe53fa25..e73d0d657ee 100644 --- a/test/helpers/projection.applyProjection.test.js +++ b/test/helpers/projection.applyProjection.test.js @@ -21,4 +21,15 @@ describe('applyProjection', function() { assert.deepEqual(applyProjection(obj, { 'nested.str2': 0 }), { str: 'test', nested: { num3: 42 } }); assert.deepEqual(applyProjection(obj, { nested: { num3: 0 } }), { str: 'test', nested: { str2: 'test2' } }); }); + + it('handles projections underneath arrays (gh-14680)', function() { + const obj = { + _id: 12, + testField: 'foo', + testArray: [{ _id: 42, field1: 'bar' }] + }; + + assert.deepEqual(applyProjection(obj, { 'testArray.field1': 1 }), { testArray: [{ field1: 'bar' }] }); + assert.deepEqual(applyProjection(obj, { 'testArray.field1': 0, _id: 0 }), { testField: 'foo', testArray: [{ _id: 42 }] }); + }); }); From 091c85dc22f6e585473320abbc239842b0d7310d Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 24 Jun 2024 15:10:44 -0400 Subject: [PATCH 3/7] fix: handle casting primitive array with $elemMatch in bulkWrite() Fix #14678 --- lib/schema/array.js | 19 +-------------- lib/schema/documentArray.js | 41 ++++++++++++++++++++++++++++++++ test/model.query.casting.test.js | 5 +++- test/model.test.js | 37 ++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 19 deletions(-) diff --git a/lib/schema/array.js b/lib/schema/array.js index e73f16d2849..67fe713a99b 100644 --- a/lib/schema/array.js +++ b/lib/schema/array.js @@ -607,24 +607,7 @@ function cast$elemMatch(val, context) { } } - // Is this an embedded discriminator and is the discriminator key set? - // If so, use the discriminator schema. See gh-7449 - const discriminatorKey = this && - this.casterConstructor && - this.casterConstructor.schema && - this.casterConstructor.schema.options && - this.casterConstructor.schema.options.discriminatorKey; - const discriminators = this && - this.casterConstructor && - this.casterConstructor.schema && - this.casterConstructor.schema.discriminators || {}; - if (discriminatorKey != null && - val[discriminatorKey] != null && - discriminators[val[discriminatorKey]] != null) { - return cast(discriminators[val[discriminatorKey]], val, null, this && this.$$context); - } - const schema = this.casterConstructor.schema ?? context.schema; - return cast(schema, val, null, this && this.$$context); + return val; } const handle = SchemaArray.prototype.$conditionalHandlers = {}; diff --git a/lib/schema/documentArray.js b/lib/schema/documentArray.js index d35c5dfbf10..a10d2ec76b1 100644 --- a/lib/schema/documentArray.js +++ b/lib/schema/documentArray.js @@ -11,9 +11,11 @@ const SchemaArray = require('./array'); const SchemaDocumentArrayOptions = require('../options/schemaDocumentArrayOptions'); const SchemaType = require('../schemaType'); +const cast = require('../cast'); const discriminator = require('../helpers/model/discriminator'); const handleIdOption = require('../helpers/schema/handleIdOption'); const handleSpreadDoc = require('../helpers/document/handleSpreadDoc'); +const isOperator = require('../helpers/query/isOperator'); const utils = require('../utils'); const getConstructor = require('../helpers/discriminator/getConstructor'); const InvalidSchemaOptionError = require('../error/invalidSchemaOption'); @@ -114,6 +116,7 @@ SchemaDocumentArray.options = { castNonArrays: true }; SchemaDocumentArray.prototype = Object.create(SchemaArray.prototype); SchemaDocumentArray.prototype.constructor = SchemaDocumentArray; SchemaDocumentArray.prototype.OptionsConstructor = SchemaDocumentArrayOptions; +SchemaDocumentArray.prototype.$conditionalHandlers = { ...SchemaArray.prototype.$conditionalHandlers }; /*! * ignore @@ -609,6 +612,44 @@ SchemaDocumentArray.setters = []; SchemaDocumentArray.get = SchemaType.get; +/*! + * Handle casting $elemMatch operators + */ + +SchemaDocumentArray.prototype.$conditionalHandlers.$elemMatch = cast$elemMatch; + +function cast$elemMatch(val, context) { + const keys = Object.keys(val); + const numKeys = keys.length; + for (let i = 0; i < numKeys; ++i) { + const key = keys[i]; + const value = val[key]; + if (isOperator(key) && value != null) { + val[key] = this.castForQuery(key, value, context); + } + } + + // Is this an embedded discriminator and is the discriminator key set? + // If so, use the discriminator schema. See gh-7449 + const discriminatorKey = this && + this.casterConstructor && + this.casterConstructor.schema && + this.casterConstructor.schema.options && + this.casterConstructor.schema.options.discriminatorKey; + const discriminators = this && + this.casterConstructor && + this.casterConstructor.schema && + this.casterConstructor.schema.discriminators || {}; + if (discriminatorKey != null && + val[discriminatorKey] != null && + discriminators[val[discriminatorKey]] != null) { + return cast(discriminators[val[discriminatorKey]], val, null, this && this.$$context); + } + + const schema = this.casterConstructor.schema ?? context.schema; + return cast(schema, val, null, this && this.$$context); +} + /*! * Module exports. */ diff --git a/test/model.query.casting.test.js b/test/model.query.casting.test.js index c7156959fc5..7f0e863bfe7 100644 --- a/test/model.query.casting.test.js +++ b/test/model.query.casting.test.js @@ -453,7 +453,10 @@ describe('model query casting', function() { const id = post._id.toString(); await post.save(); - const doc = await BlogPostB.findOne({ _id: id, comments: { $not: { $elemMatch: { _id: commentId.toString() } } } }); + const doc = await BlogPostB.findOne({ + _id: id, + comments: { $not: { $elemMatch: { _id: commentId.toString() } } } + }); assert.equal(doc, null); }); diff --git a/test/model.test.js b/test/model.test.js index d5937654a45..3d884fec59b 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -4470,6 +4470,43 @@ describe('Model', function() { assert.equal(err.validationErrors[0].path, 'age'); assert.equal(err.results[0].path, 'age'); }); + + it('casts $elemMatch filter (gh-14678)', async function() { + const schema = new mongoose.Schema({ + name: String, + ids: [String] + }); + const TestModel = db.model('Test', schema); + + const { _id } = await TestModel.create({ ids: ['1'] }); + await TestModel.bulkWrite([ + { + updateOne: { + filter: { + ids: { + $elemMatch: { + $in: [1] + } + } + }, + update: { + $set: { + name: 'test' + }, + $addToSet: { + ids: { + $each: [1, '2', 3] + } + } + } + } + } + ]); + + const doc = await TestModel.findById(_id).orFail(); + assert.strictEqual(doc.name, 'test'); + assert.deepStrictEqual(doc.ids, ['1', '2', '3']); + }); }); it('deleteOne with cast error (gh-5323)', async function() { From 561a5802e89b695abbbcb2e4cebfd235c480a684 Mon Sep 17 00:00:00 2001 From: tt-public <48979099+tt-public@users.noreply.github.com> Date: Tue, 25 Jun 2024 23:39:10 +0900 Subject: [PATCH 4/7] test: add type test for Connection.withSession --- test/types/connection.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/types/connection.test.ts b/test/types/connection.test.ts index c5979663a20..bd099dec63a 100644 --- a/test/types/connection.test.ts +++ b/test/types/connection.test.ts @@ -54,6 +54,11 @@ expectType>(conn.transaction(async(res) => { return 'a'; }, { readConcern: 'majority' })); +expectType>(conn.withSession(async(res) => { + expectType(res); + return 'a'; +})); + expectError(conn.user = 'invalid'); expectError(conn.pass = 'invalid'); expectError(conn.host = 'invalid'); From cc63f6955ca4aa7b43b9c6ea880d55bbc01eecb4 Mon Sep 17 00:00:00 2001 From: tt-public <48979099+tt-public@users.noreply.github.com> Date: Tue, 25 Jun 2024 23:40:23 +0900 Subject: [PATCH 5/7] feat: fix return type of Connection.withSession --- types/connection.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/connection.d.ts b/types/connection.d.ts index 2f47bdc84e5..1ed08ad89e3 100644 --- a/types/connection.d.ts +++ b/types/connection.d.ts @@ -247,7 +247,7 @@ declare module 'mongoose' { /** Watches the entire underlying database for changes. Similar to [`Model.watch()`](/docs/api/model.html#model_Model-watch). */ watch(pipeline?: Array, options?: mongodb.ChangeStreamOptions): mongodb.ChangeStream; - withSession(executor: (session: ClientSession) => Promise): T; + withSession(executor: (session: ClientSession) => Promise): Promise; } } From 8b05872227dcddf2202d6652eaed1f6fd2b7530c Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 25 Jun 2024 13:01:35 -0400 Subject: [PATCH 6/7] style: fix lint --- test/document.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/document.test.js b/test/document.test.js index 3ca3d5c96f3..a160f2edb62 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -2570,7 +2570,7 @@ describe('document', function() { type: String, required: true }, - testArray: [embedSchema], + testArray: [embedSchema] }); const TestModel = db.model('Test', testSchema); From 61368461263520d4ccb2842ed30ae138bc797773 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 25 Jun 2024 13:50:26 -0400 Subject: [PATCH 7/7] fix(query): remove `count()` and `findOneAndRemove()` from query chaining Fix #14689 --- lib/query.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/query.js b/lib/query.js index 5bb3ee9a611..c5c539f63be 100644 --- a/lib/query.js +++ b/lib/query.js @@ -153,6 +153,12 @@ function Query(conditions, options, model, collection) { Query.prototype = new mquery(); Query.prototype.constructor = Query; + +// Remove some legacy methods that we removed in Mongoose 8, but +// are still in mquery 5. +Query.prototype.count = undefined; +Query.prototype.findOneAndRemove = undefined; + Query.base = mquery.prototype; /*!