From 7f164df3a5dd3cb385df009300f8bee381db4245 Mon Sep 17 00:00:00 2001 From: hasezoey Date: Fri, 16 Jun 2023 12:41:07 +0200 Subject: [PATCH 01/34] types: augment bson.ObjectId instead of adding on own type fixes #12537 --- test/types/mongo.test.ts | 15 +++++++++++++++ types/augmentations.d.ts | 9 +++++++++ types/index.d.ts | 1 + types/types.d.ts | 1 - 4 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 types/augmentations.d.ts diff --git a/test/types/mongo.test.ts b/test/types/mongo.test.ts index 320ae24501b..f622ca6598c 100644 --- a/test/types/mongo.test.ts +++ b/test/types/mongo.test.ts @@ -1,4 +1,19 @@ import * as mongoose from 'mongoose'; import { expectType } from 'tsd'; +import * as bson from 'bson'; import GridFSBucket = mongoose.mongo.GridFSBucket; + +function gh12537() { + const schema = new mongoose.Schema({ test: String }); + const model = mongoose.model('Test', schema); + + const doc = new model({}); + + const v = new bson.ObjectId('somehex'); + expectType(v._id.toHexString()); + + doc._id = new bson.ObjectId('somehex'); +} + +gh12537(); diff --git a/types/augmentations.d.ts b/types/augmentations.d.ts new file mode 100644 index 00000000000..82aca589ff8 --- /dev/null +++ b/types/augmentations.d.ts @@ -0,0 +1,9 @@ +// this import is required so that types get merged instead of completely overwritten +import 'bson'; + +declare module 'bson' { + interface ObjectId { + /** Mongoose automatically adds a conveniency "_id" getter on the base ObjectId class */ + _id: this; + } +} diff --git a/types/index.d.ts b/types/index.d.ts index cfed86c51c3..ec716fa29d3 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -22,6 +22,7 @@ /// /// /// +/// declare class NativeDate extends global.Date { } diff --git a/types/types.d.ts b/types/types.d.ts index 29652243403..55b48138ad3 100644 --- a/types/types.d.ts +++ b/types/types.d.ts @@ -80,7 +80,6 @@ declare module 'mongoose' { } class ObjectId extends mongodb.ObjectId { - _id: this; } class Subdocument extends Document { From 32a84b7a65cca90c1355ce6cd218f89a92ce1e4d Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Fri, 16 Jun 2023 13:09:54 -0400 Subject: [PATCH 02/34] id setter virtual --- lib/helpers/schema/idGetter.js | 13 ++++++++++++- test/document.test.js | 12 ++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/lib/helpers/schema/idGetter.js b/lib/helpers/schema/idGetter.js index 31ea2ec8659..d521fe32240 100644 --- a/lib/helpers/schema/idGetter.js +++ b/lib/helpers/schema/idGetter.js @@ -12,8 +12,8 @@ module.exports = function addIdGetter(schema) { if (!autoIdGetter) { return schema; } - schema.virtual('id').get(idGetter); + schema.virtual('id').set(idSetter); return schema; }; @@ -30,3 +30,14 @@ function idGetter() { return null; } + +/** + * + * @param {String} v the id to set + * @api private + */ + +function idSetter(v) { + this._id = v; + return; +} diff --git a/test/document.test.js b/test/document.test.js index 1e6c9ef7afd..ec6c496d043 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -12212,6 +12212,18 @@ describe('document', function() { const fromDb = await Test.findById(x._id).lean(); assert.equal(fromDb.c.x.y, 1); }); + it('can change the value of the id property on documents gh-10096', async function() { + const testSchema = new Schema({ + name: String + }); + const Test = db.model('Test', testSchema); + const doc = new Test({ name: 'Test Testerson ' }); + const oldVal = doc.id; + doc.id = '648b8aa6a97549b03835c0b3'; + await doc.save(); + assert.notEqual(oldVal, doc.id); + assert.equal(doc.id, '648b8aa6a97549b03835c0b3'); + }); }); describe('Check if instance function that is supplied in schema option is availabe', function() { From 51dbb9aa3ac943d392c9ea698e0426ff83830a53 Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Fri, 16 Jun 2023 13:57:48 -0400 Subject: [PATCH 03/34] fix: failing test --- lib/helpers/schema/idGetter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/helpers/schema/idGetter.js b/lib/helpers/schema/idGetter.js index d521fe32240..ba5ef1011c2 100644 --- a/lib/helpers/schema/idGetter.js +++ b/lib/helpers/schema/idGetter.js @@ -39,5 +39,5 @@ function idGetter() { function idSetter(v) { this._id = v; - return; + return v; } From a73af19384300e76af32c2218fb2595b70cd242f Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 18 Jun 2023 19:23:48 -0400 Subject: [PATCH 04/34] feat(query): delay converting documents into POJOs until query execution, allow querying subdocuments with defaults disabled Fix #13512 --- lib/cast.js | 4 ++-- lib/query.js | 18 +++--------------- lib/schema/SubdocumentPath.js | 6 +++++- test/query.test.js | 30 ++++++++++++++++++++++++++++++ 4 files changed, 40 insertions(+), 18 deletions(-) diff --git a/lib/cast.js b/lib/cast.js index 278cc1f00cf..61b1f8caf13 100644 --- a/lib/cast.js +++ b/lib/cast.js @@ -10,12 +10,12 @@ const Types = require('./schema/index'); const cast$expr = require('./helpers/query/cast$expr'); const castTextSearch = require('./schema/operators/text'); const get = require('./helpers/get'); -const getConstructorName = require('./helpers/getConstructorName'); const getSchemaDiscriminatorByValue = require('./helpers/discriminator/getSchemaDiscriminatorByValue'); const isOperator = require('./helpers/query/isOperator'); const util = require('util'); const isObject = require('./helpers/isObject'); const isMongooseObject = require('./helpers/isMongooseObject'); +const utils = require('./utils'); const ALLOWED_GEOWITHIN_GEOJSON_TYPES = ['Polygon', 'MultiPolygon']; @@ -291,7 +291,7 @@ module.exports = function cast(schema, obj, options, context) { } } else if (val == null) { continue; - } else if (getConstructorName(val) === 'Object') { + } else if (utils.isPOJO(val)) { any$conditionals = Object.keys(val).some(isOperator); if (!any$conditionals) { diff --git a/lib/query.js b/lib/query.js index 4ba303b5e20..1699b2f0aa0 100644 --- a/lib/query.js +++ b/lib/query.js @@ -24,6 +24,7 @@ const getDiscriminatorByValue = require('./helpers/discriminator/getDiscriminato const hasDollarKeys = require('./helpers/query/hasDollarKeys'); const helpers = require('./queryhelpers'); const immediate = require('./helpers/immediate'); +const internalToObjectOptions = require('./options').internalToObjectOptions; const isExclusive = require('./helpers/projection/isExclusive'); const isInclusive = require('./helpers/projection/isInclusive'); const isPathSelectedInclusive = require('./helpers/projection/isPathSelectedInclusive'); @@ -2319,8 +2320,6 @@ Query.prototype.find = function(conditions) { this.op = 'find'; - conditions = utils.toObject(conditions); - if (mquery.canMerge(conditions)) { this.merge(conditions); @@ -2392,6 +2391,8 @@ Query.prototype.merge = function(source) { utils.merge(this._conditions, { _id: source }, opts); return this; + } else if (source && source.$__) { + source = source.toObject(internalToObjectOptions); } opts.omit = {}; @@ -2560,9 +2561,6 @@ Query.prototype.findOne = function(conditions, projection, options) { this.op = 'findOne'; this._validateOp(); - // make sure we don't send in the whole Document to merge() - conditions = utils.toObject(conditions); - if (options) { this.setOptions(options); } @@ -2726,8 +2724,6 @@ Query.prototype.count = function(filter) { this.op = 'count'; this._validateOp(); - filter = utils.toObject(filter); - if (mquery.canMerge(filter)) { this.merge(filter); } @@ -2822,8 +2818,6 @@ Query.prototype.countDocuments = function(conditions, options) { this.op = 'countDocuments'; this._validateOp(); - conditions = utils.toObject(conditions); - if (mquery.canMerge(conditions)) { this.merge(conditions); } @@ -2886,7 +2880,6 @@ Query.prototype.distinct = function(field, conditions) { this.op = 'distinct'; this._validateOp(); - conditions = utils.toObject(conditions); if (mquery.canMerge(conditions)) { this.merge(conditions); @@ -2981,8 +2974,6 @@ Query.prototype.deleteOne = function deleteOne(filter, options) { this.op = 'deleteOne'; this.setOptions(options); - filter = utils.toObject(filter); - if (mquery.canMerge(filter)) { this.merge(filter); @@ -3058,8 +3049,6 @@ Query.prototype.deleteMany = function(filter, options) { this.setOptions(options); this.op = 'deleteMany'; - filter = utils.toObject(filter); - if (mquery.canMerge(filter)) { this.merge(filter); @@ -4170,7 +4159,6 @@ function _update(query, op, filter, doc, options, callback) { // make sure we don't send in the whole Document to merge() query.op = op; query._validateOp(); - filter = utils.toObject(filter); doc = doc || {}; // strict is an option used in the update checking, make sure it gets set diff --git a/lib/schema/SubdocumentPath.js b/lib/schema/SubdocumentPath.js index cf15af3400e..0ff425d345b 100644 --- a/lib/schema/SubdocumentPath.js +++ b/lib/schema/SubdocumentPath.js @@ -212,11 +212,15 @@ SubdocumentPath.prototype.castForQuery = function($conditional, val, context, op return val; } + const Constructor = getConstructor(this.caster, val); + if (val instanceof Constructor) { + return val; + } + if (this.options.runSetters) { val = this._applySetters(val, context); } - const Constructor = getConstructor(this.caster, val); const overrideStrict = options != null && options.strict != null ? options.strict : void 0; diff --git a/test/query.test.js b/test/query.test.js index 24ef58598e8..8585c9460dd 100644 --- a/test/query.test.js +++ b/test/query.test.js @@ -4104,4 +4104,34 @@ describe('Query', function() { await Error.find().sort('-'); }, { message: 'Invalid field "" passed to sort()' }); }); + it('allows executing a find() with a subdocument with defaults disabled (gh-13512)', async function() { + const schema = mongoose.Schema({ + title: String, + bookHolder: mongoose.Schema({ + isReading: Boolean, + tags: [String] + }) + }); + const Test = db.model('Test', schema); + + const BookHolder = schema.path('bookHolder').caster; + + await Test.collection.insertOne({ + title: 'test-defaults-disabled', + bookHolder: { isReading: true } + }); + + // Create a new BookHolder subdocument, skip applying defaults + // Otherwise, default `[]` for `tags` would cause this query to + // return no results. + const bookHolder = new BookHolder( + { isReading: true }, + null, + null, + { defaults: false } + ); + const doc = await Test.findOne({ bookHolder }); + assert.ok(doc); + assert.equal(doc.title, 'test-defaults-disabled'); + }); }); From a59af89981691afbf2609f0909c6479aebee1aba Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 19 Jun 2023 10:05:05 -0400 Subject: [PATCH 05/34] Revert "Revert "feat(schema): add collectionOptions option to schemas"" --- docs/guide.md | 25 +++++++++++++++++++++++++ lib/model.js | 8 ++++++++ lib/schema.js | 1 + test/model.test.js | 13 +++++++++++++ types/schemaoptions.d.ts | 3 +++ 5 files changed, 50 insertions(+) diff --git a/docs/guide.md b/docs/guide.md index 66115c738da..ef34b4e318e 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -526,6 +526,7 @@ Valid options: - [skipVersioning](#skipVersioning) - [timestamps](#timestamps) - [storeSubdocValidationError](#storeSubdocValidationError) +- [collectionOptions](#collectionOptions) - [methods](#methods) - [query](#query-helpers) @@ -1399,6 +1400,30 @@ const Parent = mongoose.model('Parent', parentSchema); new Parent({ child: {} }).validateSync().errors; ``` +

+ + option: collectionOptions + +

+ +Options like [`collation`](#collation) and [`capped`](#capped) affect the options Mongoose passes to MongoDB when creating a new collection. +Mongoose schemas support most [MongoDB `createCollection()` options](https://www.mongodb.com/docs/manual/reference/method/db.createCollection/), but not all. +You can use the `collectionOptions` option to set any `createCollection()` options; Mongoose will use `collectionOptions` as the default values when calling `createCollection()` for your schema. + +```javascript +const schema = new Schema({ name: String }, { + autoCreate: false, + collectionOptions: { + capped: true, + max: 1000 + } +}); +const Test = mongoose.model('Test', schema); + +// Equivalent to `createCollection({ capped: true, max: 1000 })` +await Test.createCollection(); +``` +

With ES6 Classes

Schemas have a [`loadClass()` method](api/schema.html#schema_Schema-loadClass) diff --git a/lib/model.js b/lib/model.js index 052414d2b76..bbc4c880c44 100644 --- a/lib/model.js +++ b/lib/model.js @@ -1370,6 +1370,14 @@ Model.createCollection = async function createCollection(options) { throw new MongooseError('Model.createCollection() no longer accepts a callback'); } + const collectionOptions = this && + this.schema && + this.schema.options && + this.schema.options.collectionOptions; + if (collectionOptions != null) { + options = Object.assign({}, collectionOptions, options); + } + const schemaCollation = this && this.schema && this.schema.options && diff --git a/lib/schema.js b/lib/schema.js index 0269d1bc0f7..231761e9b16 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -80,6 +80,7 @@ let id = 0; * - [timestamps](https://mongoosejs.com/docs/guide.html#timestamps): object or boolean - defaults to `false`. If true, Mongoose adds `createdAt` and `updatedAt` properties to your schema and manages those properties for you. * - [pluginTags](https://mongoosejs.com/docs/guide.html#pluginTags): array of strings - defaults to `undefined`. If set and plugin called with `tags` option, will only apply that plugin to schemas with a matching tag. * - [virtuals](https://mongoosejs.com/docs/tutorials/virtuals.html#virtuals-via-schema-options): object - virtuals to define, alias for [`.virtual`](https://mongoosejs.com/docs/api/schema.html#Schema.prototype.virtual()) + * - [collectionOptions]: object with options passed to [`createCollection()`](https://www.mongodb.com/docs/manual/reference/method/db.createCollection/) when calling `Model.createCollection()` or `autoCreate` set to true. * * #### Options for Nested Schemas: * diff --git a/test/model.test.js b/test/model.test.js index 2042b5e8b2b..332cfffbfb4 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -7028,6 +7028,19 @@ describe('Model', function() { assert(bypass); }); }); + + it('respects schema-level `collectionOptions` for setting options to createCollection()', async function() { + const testSchema = new Schema({ + name: String + }, { collectionOptions: { capped: true, size: 1024 } }); + const TestModel = db.model('Test', testSchema); + await TestModel.init(); + await TestModel.collection.drop(); + await TestModel.createCollection(); + + const isCapped = await TestModel.collection.isCapped(); + assert.ok(isCapped); + }); }); diff --git a/types/schemaoptions.d.ts b/types/schemaoptions.d.ts index 93af4489d31..f4f5c434c77 100644 --- a/types/schemaoptions.d.ts +++ b/types/schemaoptions.d.ts @@ -49,6 +49,9 @@ declare module 'mongoose' { /** Sets a default collation for every query and aggregation. */ collation?: mongodb.CollationOptions; + /** Arbitrary options passed to `createCollection()` */ + collectionOptions?: mongodb.CreateCollectionOptions; + /** The timeseries option to use when creating the model's collection. */ timeseries?: mongodb.TimeSeriesCollectionOptions; From ffa69e37201f262fd67ef951fb77dd3da278ff63 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 21 Jun 2023 11:50:56 -0400 Subject: [PATCH 06/34] fix(document): allow setting keys with dots in mixed paths underneath nested paths Fix #13530 --- lib/document.js | 3 -- .../path/flattenObjectWithDottedPaths.js | 39 ------------------- test/document.test.js | 21 ++++++++++ 3 files changed, 21 insertions(+), 42 deletions(-) delete mode 100644 lib/helpers/path/flattenObjectWithDottedPaths.js diff --git a/lib/document.js b/lib/document.js index 81d51a2ce3e..333ba114393 100644 --- a/lib/document.js +++ b/lib/document.js @@ -22,7 +22,6 @@ const clone = require('./helpers/clone'); const compile = require('./helpers/document/compile').compile; const defineKey = require('./helpers/document/compile').defineKey; const flatten = require('./helpers/common').flatten; -const flattenObjectWithDottedPaths = require('./helpers/path/flattenObjectWithDottedPaths'); const get = require('./helpers/get'); const getEmbeddedDiscriminatorPath = require('./helpers/document/getEmbeddedDiscriminatorPath'); const getKeysInSchemaOrder = require('./helpers/schema/getKeysInSchemaOrder'); @@ -473,8 +472,6 @@ function $applyDefaultsToNested(val, path, doc) { return; } - flattenObjectWithDottedPaths(val); - const paths = Object.keys(doc.$__schema.paths); const plen = paths.length; diff --git a/lib/helpers/path/flattenObjectWithDottedPaths.js b/lib/helpers/path/flattenObjectWithDottedPaths.js deleted file mode 100644 index 2771796d082..00000000000 --- a/lib/helpers/path/flattenObjectWithDottedPaths.js +++ /dev/null @@ -1,39 +0,0 @@ -'use strict'; - -const MongooseError = require('../../error/mongooseError'); -const isMongooseObject = require('../isMongooseObject'); -const setDottedPath = require('../path/setDottedPath'); -const util = require('util'); - -/** - * Given an object that may contain dotted paths, flatten the paths out. - * For example: `flattenObjectWithDottedPaths({ a: { 'b.c': 42 } })` => `{ a: { b: { c: 42 } } }` - */ - -module.exports = function flattenObjectWithDottedPaths(obj) { - if (obj == null || typeof obj !== 'object' || Array.isArray(obj)) { - return; - } - // Avoid Mongoose docs, like docs and maps, because these may cause infinite recursion - if (isMongooseObject(obj)) { - return; - } - const keys = Object.keys(obj); - for (const key of keys) { - const val = obj[key]; - if (key.indexOf('.') !== -1) { - try { - delete obj[key]; - setDottedPath(obj, key, val); - } catch (err) { - if (!(err instanceof TypeError)) { - throw err; - } - throw new MongooseError(`Conflicting dotted paths when setting document path, key: "${key}", value: ${util.inspect(val)}`); - } - continue; - } - - flattenObjectWithDottedPaths(obj[key]); - } -}; diff --git a/test/document.test.js b/test/document.test.js index 1e6c9ef7afd..7c46635dfd6 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -12212,6 +12212,27 @@ describe('document', function() { const fromDb = await Test.findById(x._id).lean(); assert.equal(fromDb.c.x.y, 1); }); + + it('should allow storing keys with dots in name in mixed under nested (gh-13530)', async function() { + const TestModelSchema = new mongoose.Schema({ + metadata: + { + labels: mongoose.Schema.Types.Mixed + } + }); + const TestModel = db.model('Test', TestModelSchema); + const { _id } = await TestModel.create({ + metadata: { + labels: { 'my.label.com': 'true' } + } + }); + const doc = await TestModel.findById(_id).lean(); + assert.deepStrictEqual(doc.metadata, { + labels: { + 'my.label.com': 'true' + } + }); + }); }); describe('Check if instance function that is supplied in schema option is availabe', function() { From dd53fc7aa31b4de93634802eee1dce14a49c53c5 Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Wed, 21 Jun 2023 18:03:23 -0400 Subject: [PATCH 07/34] begin implementation --- lib/schematype.js | 12 ++++++------ test/schematype.test.js | 13 +++++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/schematype.js b/lib/schematype.js index 76664b77529..3bef257f9eb 100644 --- a/lib/schematype.js +++ b/lib/schematype.js @@ -48,7 +48,9 @@ function SchemaType(path, options, instance) { this.getters = this.constructor.hasOwnProperty('getters') ? this.constructor.getters.slice() : []; - this.setters = []; + this.setters = this.constructor.hasOwnProperty('setters') ? + this.constructor.setters.slice() : + []; this.splitPath(); @@ -295,11 +297,9 @@ SchemaType.prototype.cast = function cast() { * @api public */ -SchemaType.set = function set(option, value) { - if (!this.hasOwnProperty('defaultOptions')) { - this.defaultOptions = Object.assign({}, this.defaultOptions); - } - this.defaultOptions[option] = value; +SchemaType.set = function(setter) { + this.setters = this.hasOwnProperty('setters') ? this.setters : []; + this.setters.push(setter); }; /** diff --git a/test/schematype.test.js b/test/schematype.test.js index 7cb806cada8..3ed885a06a2 100644 --- a/test/schematype.test.js +++ b/test/schematype.test.js @@ -211,6 +211,19 @@ describe('schematype', function() { it('SchemaType.set, is a function', () => { assert.equal(typeof mongoose.SchemaType.set, 'function'); }); + it('should allow setting values to a given property gh-13510', async function() { + assert(mongoose.Schema.Types.Date); + mongoose.Schema.Types.Date.setters.push(v => typeof v === 'string' && /^\d{8}$/.test(v) ? new Date(v.slice(0, 4), +v.slice(4, 6) - 1, v.slice(6, 8)) : v); + const testSchema = new Schema({ + myDate: Date + }); + const Test = mongoose.model('Test', testSchema); + const doc = new Test(); + doc.myDate = '20220601'; + await doc.save(); + console.log('what is doc', doc); + assert(doc.myDate); + }); }); const typesToTest = Object.values(mongoose.SchemaTypes). From f8b1511987cd50365b351a70bb62510d182f77bf Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Thu, 22 Jun 2023 16:36:03 -0400 Subject: [PATCH 08/34] add test --- lib/model.js | 1 - test/schematype.test.js | 15 +++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/model.js b/lib/model.js index bbc4c880c44..c9741ce6675 100644 --- a/lib/model.js +++ b/lib/model.js @@ -320,7 +320,6 @@ Model.prototype.$__handleSave = function(options, callback) { _setIsNew(this, false); // Make it possible to retry the insert this.$__.inserting = true; - return; } diff --git a/test/schematype.test.js b/test/schematype.test.js index 3ed885a06a2..f91630ba32b 100644 --- a/test/schematype.test.js +++ b/test/schematype.test.js @@ -211,18 +211,17 @@ describe('schematype', function() { it('SchemaType.set, is a function', () => { assert.equal(typeof mongoose.SchemaType.set, 'function'); }); - it('should allow setting values to a given property gh-13510', async function() { - assert(mongoose.Schema.Types.Date); - mongoose.Schema.Types.Date.setters.push(v => typeof v === 'string' && /^\d{8}$/.test(v) ? new Date(v.slice(0, 4), +v.slice(4, 6) - 1, v.slice(6, 8)) : v); - const testSchema = new Schema({ + it('should allow setting values to a given property gh-13510', function() { + const m = new mongoose.Mongoose(); + m.SchemaTypes.Date.set(v => typeof v === 'string' && /^\d{8}$/.test(v) ? new Date(v.slice(0, 4), +v.slice(4, 6) - 1, v.slice(6, 8)) : v); + const testSchema = new m.Schema({ myDate: Date }); - const Test = mongoose.model('Test', testSchema); + const Test = m.model('Test', testSchema); const doc = new Test(); doc.myDate = '20220601'; - await doc.save(); - console.log('what is doc', doc); - assert(doc.myDate); + doc.save().then(); + assert(doc.myDate instanceof Date); }); }); From ee3e7c9f8fbecc01d0ba874c859426eddc7f826c Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Thu, 22 Jun 2023 17:37:00 -0400 Subject: [PATCH 09/34] add `setters = []` to all relevant types --- lib/schema/SubdocumentPath.js | 2 ++ lib/schema/array.js | 2 ++ lib/schema/bigint.js | 2 ++ lib/schema/boolean.js | 2 ++ lib/schema/buffer.js | 2 ++ lib/schema/date.js | 2 ++ lib/schema/decimal128.js | 2 ++ lib/schema/documentarray.js | 2 ++ lib/schema/mixed.js | 2 ++ lib/schema/number.js | 2 ++ lib/schema/objectid.js | 2 ++ lib/schema/string.js | 2 ++ lib/schema/uuid.js | 2 ++ test/schematype.test.js | 2 +- 14 files changed, 27 insertions(+), 1 deletion(-) diff --git a/lib/schema/SubdocumentPath.js b/lib/schema/SubdocumentPath.js index 0ff425d345b..bb9b6022562 100644 --- a/lib/schema/SubdocumentPath.js +++ b/lib/schema/SubdocumentPath.js @@ -351,6 +351,8 @@ SubdocumentPath.defaultOptions = {}; SubdocumentPath.set = SchemaType.set; +SubdocumentPath.setters = []; + /** * Attaches a getter for all SubdocumentPath instances * diff --git a/lib/schema/array.js b/lib/schema/array.js index 9b292fbc3d7..4e10468a769 100644 --- a/lib/schema/array.js +++ b/lib/schema/array.js @@ -170,6 +170,8 @@ SchemaArray.defaultOptions = {}; */ SchemaArray.set = SchemaType.set; +SchemaArray.setters = []; + /** * Attaches a getter for all Array instances * diff --git a/lib/schema/bigint.js b/lib/schema/bigint.js index bea5abc61df..4c7dcb77039 100644 --- a/lib/schema/bigint.js +++ b/lib/schema/bigint.js @@ -62,6 +62,8 @@ SchemaBigInt._cast = castBigInt; SchemaBigInt.set = SchemaType.set; +SchemaBigInt.setters = []; + /** * Attaches a getter for all BigInt instances * diff --git a/lib/schema/boolean.js b/lib/schema/boolean.js index 49e4b4e67b0..316c825df45 100644 --- a/lib/schema/boolean.js +++ b/lib/schema/boolean.js @@ -65,6 +65,8 @@ SchemaBoolean._cast = castBoolean; SchemaBoolean.set = SchemaType.set; +SchemaBoolean.setters = []; + /** * Attaches a getter for all Boolean instances * diff --git a/lib/schema/buffer.js b/lib/schema/buffer.js index badb1bafabe..5bfaabcd2f6 100644 --- a/lib/schema/buffer.js +++ b/lib/schema/buffer.js @@ -70,6 +70,8 @@ SchemaBuffer._checkRequired = v => !!(v && v.length); SchemaBuffer.set = SchemaType.set; +SchemaBuffer.setters = []; + /** * Attaches a getter for all Buffer instances * diff --git a/lib/schema/date.js b/lib/schema/date.js index 41625703492..61928bb457d 100644 --- a/lib/schema/date.js +++ b/lib/schema/date.js @@ -70,6 +70,8 @@ SchemaDate._cast = castDate; SchemaDate.set = SchemaType.set; +SchemaDate.setters = []; + /** * Attaches a getter for all Date instances * diff --git a/lib/schema/decimal128.js b/lib/schema/decimal128.js index c0c79d5834c..650d78c2571 100644 --- a/lib/schema/decimal128.js +++ b/lib/schema/decimal128.js @@ -66,6 +66,8 @@ Decimal128._cast = castDecimal128; Decimal128.set = SchemaType.set; +Decimal128.setters = []; + /** * Attaches a getter for all Decimal128 instances * diff --git a/lib/schema/documentarray.js b/lib/schema/documentarray.js index 7de35055ce7..7f8a39a04d6 100644 --- a/lib/schema/documentarray.js +++ b/lib/schema/documentarray.js @@ -605,6 +605,8 @@ DocumentArrayPath.defaultOptions = {}; DocumentArrayPath.set = SchemaType.set; +DocumentArrayPath.setters = []; + /** * Attaches a getter for all DocumentArrayPath instances * diff --git a/lib/schema/mixed.js b/lib/schema/mixed.js index 5d6e40cd926..bd38a286213 100644 --- a/lib/schema/mixed.js +++ b/lib/schema/mixed.js @@ -94,6 +94,8 @@ Mixed.get = SchemaType.get; Mixed.set = SchemaType.set; +Mixed.setters = []; + /** * Casts `val` for Mixed. * diff --git a/lib/schema/number.js b/lib/schema/number.js index 48d3ff7ffda..b2695cd94a6 100644 --- a/lib/schema/number.js +++ b/lib/schema/number.js @@ -67,6 +67,8 @@ SchemaNumber.get = SchemaType.get; SchemaNumber.set = SchemaType.set; +SchemaNumber.setters = []; + /*! * ignore */ diff --git a/lib/schema/objectid.js b/lib/schema/objectid.js index 15e41eaefcd..f0a0b6be747 100644 --- a/lib/schema/objectid.js +++ b/lib/schema/objectid.js @@ -94,6 +94,8 @@ ObjectId.get = SchemaType.get; ObjectId.set = SchemaType.set; +ObjectId.setters = []; + /** * Adds an auto-generated ObjectId default if turnOn is true. * @param {Boolean} turnOn auto generated ObjectId defaults diff --git a/lib/schema/string.js b/lib/schema/string.js index bf1b6d85749..5caee2e3cfc 100644 --- a/lib/schema/string.js +++ b/lib/schema/string.js @@ -143,6 +143,8 @@ SchemaString.get = SchemaType.get; SchemaString.set = SchemaType.set; +SchemaString.setters = []; + /*! * ignore */ diff --git a/lib/schema/uuid.js b/lib/schema/uuid.js index 7656ceccec6..9de95f6cd4d 100644 --- a/lib/schema/uuid.js +++ b/lib/schema/uuid.js @@ -194,6 +194,8 @@ SchemaUUID.get = SchemaType.get; SchemaUUID.set = SchemaType.set; +SchemaUUID.setters = []; + /** * Get/set the function used to cast arbitrary values to UUIDs. * diff --git a/test/schematype.test.js b/test/schematype.test.js index f91630ba32b..5aed8fcf536 100644 --- a/test/schematype.test.js +++ b/test/schematype.test.js @@ -213,7 +213,7 @@ describe('schematype', function() { }); it('should allow setting values to a given property gh-13510', function() { const m = new mongoose.Mongoose(); - m.SchemaTypes.Date.set(v => typeof v === 'string' && /^\d{8}$/.test(v) ? new Date(v.slice(0, 4), +v.slice(4, 6) - 1, v.slice(6, 8)) : v); + m.SchemaTypes.Date.setters.push(v => typeof v === 'string' && /^\d{8}$/.test(v) ? new Date(v.slice(0, 4), +v.slice(4, 6) - 1, v.slice(6, 8)) : v); const testSchema = new m.Schema({ myDate: Date }); From 32e4a2efef4fea3679530f20e28122f357adb352 Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Fri, 23 Jun 2023 10:27:15 -0400 Subject: [PATCH 10/34] revert `set()` changes --- lib/schematype.js | 8 +++++--- test/schematype.test.js | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/schematype.js b/lib/schematype.js index 3bef257f9eb..f5e6025510d 100644 --- a/lib/schematype.js +++ b/lib/schematype.js @@ -297,9 +297,11 @@ SchemaType.prototype.cast = function cast() { * @api public */ -SchemaType.set = function(setter) { - this.setters = this.hasOwnProperty('setters') ? this.setters : []; - this.setters.push(setter); +SchemaType.set = function set(option, value) { + if (!this.hasOwnProperty('defaultOptions')) { + this.defaultOptions = Object.assign({}, this.defaultOptions); + } + this.defaultOptions[option] = value; }; /** diff --git a/test/schematype.test.js b/test/schematype.test.js index 5aed8fcf536..b1e078f69e2 100644 --- a/test/schematype.test.js +++ b/test/schematype.test.js @@ -211,7 +211,7 @@ describe('schematype', function() { it('SchemaType.set, is a function', () => { assert.equal(typeof mongoose.SchemaType.set, 'function'); }); - it('should allow setting values to a given property gh-13510', function() { + it('should allow setting values to a given property gh-13510', async function() { const m = new mongoose.Mongoose(); m.SchemaTypes.Date.setters.push(v => typeof v === 'string' && /^\d{8}$/.test(v) ? new Date(v.slice(0, 4), +v.slice(4, 6) - 1, v.slice(6, 8)) : v); const testSchema = new m.Schema({ @@ -220,7 +220,7 @@ describe('schematype', function() { const Test = m.model('Test', testSchema); const doc = new Test(); doc.myDate = '20220601'; - doc.save().then(); + await doc.save(); assert(doc.myDate instanceof Date); }); }); From 8c3ac85df3086a980bf5d0b185925adf1a00f5fc Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Fri, 23 Jun 2023 12:05:45 -0400 Subject: [PATCH 11/34] remove async/await from test --- test/schematype.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/schematype.test.js b/test/schematype.test.js index b1e078f69e2..5aed8fcf536 100644 --- a/test/schematype.test.js +++ b/test/schematype.test.js @@ -211,7 +211,7 @@ describe('schematype', function() { it('SchemaType.set, is a function', () => { assert.equal(typeof mongoose.SchemaType.set, 'function'); }); - it('should allow setting values to a given property gh-13510', async function() { + it('should allow setting values to a given property gh-13510', function() { const m = new mongoose.Mongoose(); m.SchemaTypes.Date.setters.push(v => typeof v === 'string' && /^\d{8}$/.test(v) ? new Date(v.slice(0, 4), +v.slice(4, 6) - 1, v.slice(6, 8)) : v); const testSchema = new m.Schema({ @@ -220,7 +220,7 @@ describe('schematype', function() { const Test = m.model('Test', testSchema); const doc = new Test(); doc.myDate = '20220601'; - await doc.save(); + doc.save().then(); assert(doc.myDate instanceof Date); }); }); From 9c7618113e4809d4a4934cb79e67496b79cb87b6 Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Fri, 23 Jun 2023 12:09:52 -0400 Subject: [PATCH 12/34] add async/await back --- test/schematype.test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/schematype.test.js b/test/schematype.test.js index 5aed8fcf536..a87e2727740 100644 --- a/test/schematype.test.js +++ b/test/schematype.test.js @@ -211,16 +211,18 @@ describe('schematype', function() { it('SchemaType.set, is a function', () => { assert.equal(typeof mongoose.SchemaType.set, 'function'); }); - it('should allow setting values to a given property gh-13510', function() { + it('should allow setting values to a given property gh-13510', async function() { const m = new mongoose.Mongoose(); + await m.connect('mongodb://127.0.0.1:27017/gh13510'); m.SchemaTypes.Date.setters.push(v => typeof v === 'string' && /^\d{8}$/.test(v) ? new Date(v.slice(0, 4), +v.slice(4, 6) - 1, v.slice(6, 8)) : v); const testSchema = new m.Schema({ myDate: Date }); const Test = m.model('Test', testSchema); + await Test.deleteMany({}); const doc = new Test(); doc.myDate = '20220601'; - doc.save().then(); + await doc.save(); assert(doc.myDate instanceof Date); }); }); From 7a4a6780b6b0b4dc89a3989d2aaced7f801a169c Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Fri, 23 Jun 2023 12:14:41 -0400 Subject: [PATCH 13/34] close connection --- test/schematype.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/schematype.test.js b/test/schematype.test.js index a87e2727740..a2660e9f517 100644 --- a/test/schematype.test.js +++ b/test/schematype.test.js @@ -223,6 +223,7 @@ describe('schematype', function() { const doc = new Test(); doc.myDate = '20220601'; await doc.save(); + await m.connections[0].close(); assert(doc.myDate instanceof Date); }); }); From 66730c4d29dfbe517dacfe323b4b23f610f77551 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 25 Jun 2023 14:27:52 -0400 Subject: [PATCH 14/34] refactor: move all MongoDB-specific connection logic into driver layer, add `createClient()` method to handle creating MongoClient --- lib/connection.js | 209 +----------------- lib/drivers/node-mongodb-native/connection.js | 209 ++++++++++++++++++ test/timestamps.test.js | 5 +- 3 files changed, 221 insertions(+), 202 deletions(-) diff --git a/lib/connection.js b/lib/connection.js index 66035bbfe4b..83abd2d62cd 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -16,10 +16,7 @@ const clone = require('./helpers/clone'); const driver = require('./driver'); const get = require('./helpers/get'); const immediate = require('./helpers/immediate'); -const mongodb = require('mongodb'); -const pkg = require('../package.json'); const utils = require('./utils'); -const processConnectionOptions = require('./helpers/processConnectionOptions'); const CreateCollectionsError = require('./error/createCollectionsError'); const arrayAtomicsSymbol = require('./helpers/symbols').arrayAtomicsSymbol; @@ -739,7 +736,7 @@ Connection.prototype.openUri = async function openUri(uri, options) { throw err; } - this.$initialConnection = _createMongoClient(this, uri, options). + this.$initialConnection = this.createClient(this, uri, options). then(() => this). catch(err => { this.readyState = STATES.disconnected; @@ -796,184 +793,6 @@ function _handleConnectionErrors(err) { return err; } -/*! - * ignore - */ - -async function _createMongoClient(conn, uri, options) { - if (typeof uri !== 'string') { - throw new MongooseError('The `uri` parameter to `openUri()` must be a ' + - `string, got "${typeof uri}". Make sure the first parameter to ` + - '`mongoose.connect()` or `mongoose.createConnection()` is a string.'); - } - - if (conn._destroyCalled) { - throw new MongooseError( - 'Connection has been closed and destroyed, and cannot be used for re-opening the connection. ' + - 'Please create a new connection with `mongoose.createConnection()` or `mongoose.connect()`.' - ); - } - - if (conn.readyState === STATES.connecting || conn.readyState === STATES.connected) { - if (conn._connectionString !== uri) { - throw new MongooseError('Can\'t call `openUri()` on an active connection with ' + - 'different connection strings. Make sure you aren\'t calling `mongoose.connect()` ' + - 'multiple times. See: https://mongoosejs.com/docs/connections.html#multiple_connections'); - } - } - - options = processConnectionOptions(uri, options); - - if (options) { - - const autoIndex = options.config && options.config.autoIndex != null ? - options.config.autoIndex : - options.autoIndex; - if (autoIndex != null) { - conn.config.autoIndex = autoIndex !== false; - delete options.config; - delete options.autoIndex; - } - - if ('autoCreate' in options) { - conn.config.autoCreate = !!options.autoCreate; - delete options.autoCreate; - } - - if ('sanitizeFilter' in options) { - conn.config.sanitizeFilter = options.sanitizeFilter; - delete options.sanitizeFilter; - } - - // Backwards compat - if (options.user || options.pass) { - options.auth = options.auth || {}; - options.auth.username = options.user; - options.auth.password = options.pass; - - conn.user = options.user; - conn.pass = options.pass; - } - delete options.user; - delete options.pass; - - if (options.bufferCommands != null) { - conn.config.bufferCommands = options.bufferCommands; - delete options.bufferCommands; - } - } else { - options = {}; - } - - conn._connectionOptions = options; - const dbName = options.dbName; - if (dbName != null) { - conn.$dbName = dbName; - } - delete options.dbName; - - if (!utils.hasUserDefinedProperty(options, 'driverInfo')) { - options.driverInfo = { - name: 'Mongoose', - version: pkg.version - }; - } - - conn.readyState = STATES.connecting; - conn._connectionString = uri; - - let client; - try { - client = new mongodb.MongoClient(uri, options); - } catch (error) { - conn.readyState = STATES.disconnected; - throw error; - } - conn.client = client; - - client.setMaxListeners(0); - await client.connect(); - - _setClient(conn, client, options, dbName); - - for (const db of conn.otherDbs) { - _setClient(db, client, {}, db.name); - } - return conn; -} - -/*! - * ignore - */ - -function _setClient(conn, client, options, dbName) { - const db = dbName != null ? client.db(dbName) : client.db(); - conn.db = db; - conn.client = client; - conn.host = client && - client.s && - client.s.options && - client.s.options.hosts && - client.s.options.hosts[0] && - client.s.options.hosts[0].host || void 0; - conn.port = client && - client.s && - client.s.options && - client.s.options.hosts && - client.s.options.hosts[0] && - client.s.options.hosts[0].port || void 0; - conn.name = dbName != null ? dbName : client && client.s && client.s.options && client.s.options.dbName || void 0; - conn._closeCalled = client._closeCalled; - - const _handleReconnect = () => { - // If we aren't disconnected, we assume this reconnect is due to a - // socket timeout. If there's no activity on a socket for - // `socketTimeoutMS`, the driver will attempt to reconnect and emit - // this event. - if (conn.readyState !== STATES.connected) { - conn.readyState = STATES.connected; - conn.emit('reconnect'); - conn.emit('reconnected'); - conn.onOpen(); - } - }; - - const type = client && - client.topology && - client.topology.description && - client.topology.description.type || ''; - - if (type === 'Single') { - client.on('serverDescriptionChanged', ev => { - const newDescription = ev.newDescription; - if (newDescription.type === 'Unknown') { - conn.readyState = STATES.disconnected; - } else { - _handleReconnect(); - } - }); - } else if (type.startsWith('ReplicaSet')) { - client.on('topologyDescriptionChanged', ev => { - // Emit disconnected if we've lost connectivity to the primary - const description = ev.newDescription; - if (conn.readyState === STATES.connected && description.type !== 'ReplicaSetWithPrimary') { - // Implicitly emits 'disconnected' - conn.readyState = STATES.disconnected; - } else if (conn.readyState === STATES.disconnected && description.type === 'ReplicaSetWithPrimary') { - _handleReconnect(); - } - }); - } - - conn.onOpen(); - - for (const i in conn.collections) { - if (utils.object.hasOwnProperty(conn.collections, i)) { - conn.collections[i].onOpen(); - } - } -} - /** * Destroy the connection. Similar to [`.close`](https://mongoosejs.com/docs/api/connection.html#Connection.prototype.close()), * but also removes the connection from Mongoose's `connections` list and prevents the @@ -1526,26 +1345,16 @@ Connection.prototype.getClient = function getClient() { * @return {Connection} this */ -Connection.prototype.setClient = function setClient(client) { - if (!(client instanceof mongodb.MongoClient)) { - throw new MongooseError('Must call `setClient()` with an instance of MongoClient'); - } - if (this.readyState !== STATES.disconnected) { - throw new MongooseError('Cannot call `setClient()` on a connection that is already connected.'); - } - if (client.topology == null) { - throw new MongooseError('Cannot call `setClient()` with a MongoClient that you have not called `connect()` on yet.'); - } - - this._connectionString = client.s.url; - _setClient(this, client, {}, client.s.options.dbName); +Connection.prototype.setClient = function setClient() { + throw new MongooseError('Connection#setClient not implemented by driver'); +}; - for (const model of Object.values(this.models)) { - // Errors handled internally, so safe to ignore error - model.init().catch(function $modelInitNoop() {}); - } +/*! + * Called internally by `openUri()` to create a MongoClient instance. + */ - return this; +Connection.prototype.createClient = function createClient() { + throw new MongooseError('Connection#createClient not implemented by driver'); }; /** diff --git a/lib/drivers/node-mongodb-native/connection.js b/lib/drivers/node-mongodb-native/connection.js index aaa3acd6160..d574f2b3059 100644 --- a/lib/drivers/node-mongodb-native/connection.js +++ b/lib/drivers/node-mongodb-native/connection.js @@ -5,8 +5,13 @@ 'use strict'; const MongooseConnection = require('../../connection'); +const MongooseError = require('../../error/index'); const STATES = require('../../connectionstate'); +const mongodb = require('mongodb'); +const pkg = require('../../../package.json'); +const processConnectionOptions = require('../../helpers/processConnectionOptions'); const setTimeout = require('../../helpers/timers').setTimeout; +const utils = require('../../utils'); /** * A [node-mongodb-native](https://github.com/mongodb/node-mongodb-native) connection implementation. @@ -154,6 +159,210 @@ NativeConnection.prototype.doClose = async function doClose(force) { return this; }; +/*! + * ignore + */ + +NativeConnection.prototype.createClient = async function createClient(conn, uri, options) { + if (typeof uri !== 'string') { + throw new MongooseError('The `uri` parameter to `openUri()` must be a ' + + `string, got "${typeof uri}". Make sure the first parameter to ` + + '`mongoose.connect()` or `mongoose.createConnection()` is a string.'); + } + + if (conn._destroyCalled) { + throw new MongooseError( + 'Connection has been closed and destroyed, and cannot be used for re-opening the connection. ' + + 'Please create a new connection with `mongoose.createConnection()` or `mongoose.connect()`.' + ); + } + + if (conn.readyState === STATES.connecting || conn.readyState === STATES.connected) { + if (conn._connectionString !== uri) { + throw new MongooseError('Can\'t call `openUri()` on an active connection with ' + + 'different connection strings. Make sure you aren\'t calling `mongoose.connect()` ' + + 'multiple times. See: https://mongoosejs.com/docs/connections.html#multiple_connections'); + } + } + + options = processConnectionOptions(uri, options); + + if (options) { + + const autoIndex = options.config && options.config.autoIndex != null ? + options.config.autoIndex : + options.autoIndex; + if (autoIndex != null) { + conn.config.autoIndex = autoIndex !== false; + delete options.config; + delete options.autoIndex; + } + + if ('autoCreate' in options) { + conn.config.autoCreate = !!options.autoCreate; + delete options.autoCreate; + } + + if ('sanitizeFilter' in options) { + conn.config.sanitizeFilter = options.sanitizeFilter; + delete options.sanitizeFilter; + } + + // Backwards compat + if (options.user || options.pass) { + options.auth = options.auth || {}; + options.auth.username = options.user; + options.auth.password = options.pass; + + conn.user = options.user; + conn.pass = options.pass; + } + delete options.user; + delete options.pass; + + if (options.bufferCommands != null) { + conn.config.bufferCommands = options.bufferCommands; + delete options.bufferCommands; + } + } else { + options = {}; + } + + conn._connectionOptions = options; + const dbName = options.dbName; + if (dbName != null) { + conn.$dbName = dbName; + } + delete options.dbName; + + if (!utils.hasUserDefinedProperty(options, 'driverInfo')) { + options.driverInfo = { + name: 'Mongoose', + version: pkg.version + }; + } + + conn.readyState = STATES.connecting; + conn._connectionString = uri; + + let client; + try { + client = new mongodb.MongoClient(uri, options); + } catch (error) { + conn.readyState = STATES.disconnected; + throw error; + } + conn.client = client; + + client.setMaxListeners(0); + await client.connect(); + + _setClient(conn, client, options, dbName); + + for (const db of conn.otherDbs) { + _setClient(db, client, {}, db.name); + } + return conn; +}; + +/*! + * ignore + */ + +NativeConnection.prototype.setClient = function setClient(client) { + if (!(client instanceof mongodb.MongoClient)) { + throw new MongooseError('Must call `setClient()` with an instance of MongoClient'); + } + if (this.readyState !== STATES.disconnected) { + throw new MongooseError('Cannot call `setClient()` on a connection that is already connected.'); + } + if (client.topology == null) { + throw new MongooseError('Cannot call `setClient()` with a MongoClient that you have not called `connect()` on yet.'); + } + + this._connectionString = client.s.url; + _setClient(this, client, {}, client.s.options.dbName); + + for (const model of Object.values(this.models)) { + // Errors handled internally, so safe to ignore error + model.init().catch(function $modelInitNoop() {}); + } + + return this; +}; + +/*! + * ignore + */ + +function _setClient(conn, client, options, dbName) { + const db = dbName != null ? client.db(dbName) : client.db(); + conn.db = db; + conn.client = client; + conn.host = client && + client.s && + client.s.options && + client.s.options.hosts && + client.s.options.hosts[0] && + client.s.options.hosts[0].host || void 0; + conn.port = client && + client.s && + client.s.options && + client.s.options.hosts && + client.s.options.hosts[0] && + client.s.options.hosts[0].port || void 0; + conn.name = dbName != null ? dbName : client && client.s && client.s.options && client.s.options.dbName || void 0; + conn._closeCalled = client._closeCalled; + + const _handleReconnect = () => { + // If we aren't disconnected, we assume this reconnect is due to a + // socket timeout. If there's no activity on a socket for + // `socketTimeoutMS`, the driver will attempt to reconnect and emit + // this event. + if (conn.readyState !== STATES.connected) { + conn.readyState = STATES.connected; + conn.emit('reconnect'); + conn.emit('reconnected'); + conn.onOpen(); + } + }; + + const type = client && + client.topology && + client.topology.description && + client.topology.description.type || ''; + + if (type === 'Single') { + client.on('serverDescriptionChanged', ev => { + const newDescription = ev.newDescription; + if (newDescription.type === 'Unknown') { + conn.readyState = STATES.disconnected; + } else { + _handleReconnect(); + } + }); + } else if (type.startsWith('ReplicaSet')) { + client.on('topologyDescriptionChanged', ev => { + // Emit disconnected if we've lost connectivity to the primary + const description = ev.newDescription; + if (conn.readyState === STATES.connected && description.type !== 'ReplicaSetWithPrimary') { + // Implicitly emits 'disconnected' + conn.readyState = STATES.disconnected; + } else if (conn.readyState === STATES.disconnected && description.type === 'ReplicaSetWithPrimary') { + _handleReconnect(); + } + }); + } + + conn.onOpen(); + + for (const i in conn.collections) { + if (utils.object.hasOwnProperty(conn.collections, i)) { + conn.collections[i].onOpen(); + } + } +} + /*! * Module exports. diff --git a/test/timestamps.test.js b/test/timestamps.test.js index 027d7601f2c..49ab3e82762 100644 --- a/test/timestamps.test.js +++ b/test/timestamps.test.js @@ -608,12 +608,13 @@ describe('timestamps', function() { }); it('should change updatedAt when findOneAndUpdate', async function() { + await Cat.deleteMany({}); await Cat.create({ name: 'test123' }); let doc = await Cat.findOne({ name: 'test123' }); const old = doc.updatedAt; + await new Promise(resolve => setTimeout(resolve, 10)); doc = await Cat.findOneAndUpdate({ name: 'test123' }, { $set: { hobby: 'fish' } }, { new: true }); - assert.ok(doc.updatedAt.getTime() > old.getTime()); - + assert.ok(doc.updatedAt.getTime() > old.getTime(), `Expected ${doc.updatedAt} > ${old}`); }); it('insertMany with createdAt off (gh-6381)', async function() { From 24a72f9504a8882375285795af2286fce14d64ef Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 25 Jun 2023 14:31:05 -0400 Subject: [PATCH 15/34] refactor: cleanup createClient() function signature --- lib/connection.js | 2 +- lib/drivers/node-mongodb-native/connection.js | 38 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/lib/connection.js b/lib/connection.js index 83abd2d62cd..48c13ccd9d2 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -736,7 +736,7 @@ Connection.prototype.openUri = async function openUri(uri, options) { throw err; } - this.$initialConnection = this.createClient(this, uri, options). + this.$initialConnection = this.createClient(uri, options). then(() => this). catch(err => { this.readyState = STATES.disconnected; diff --git a/lib/drivers/node-mongodb-native/connection.js b/lib/drivers/node-mongodb-native/connection.js index d574f2b3059..417085be12e 100644 --- a/lib/drivers/node-mongodb-native/connection.js +++ b/lib/drivers/node-mongodb-native/connection.js @@ -163,22 +163,22 @@ NativeConnection.prototype.doClose = async function doClose(force) { * ignore */ -NativeConnection.prototype.createClient = async function createClient(conn, uri, options) { +NativeConnection.prototype.createClient = async function createClient(uri, options) { if (typeof uri !== 'string') { throw new MongooseError('The `uri` parameter to `openUri()` must be a ' + `string, got "${typeof uri}". Make sure the first parameter to ` + '`mongoose.connect()` or `mongoose.createConnection()` is a string.'); } - if (conn._destroyCalled) { + if (this._destroyCalled) { throw new MongooseError( 'Connection has been closed and destroyed, and cannot be used for re-opening the connection. ' + 'Please create a new connection with `mongoose.createConnection()` or `mongoose.connect()`.' ); } - if (conn.readyState === STATES.connecting || conn.readyState === STATES.connected) { - if (conn._connectionString !== uri) { + if (this.readyState === STATES.connecting || this.readyState === STATES.connected) { + if (this._connectionString !== uri) { throw new MongooseError('Can\'t call `openUri()` on an active connection with ' + 'different connection strings. Make sure you aren\'t calling `mongoose.connect()` ' + 'multiple times. See: https://mongoosejs.com/docs/connections.html#multiple_connections'); @@ -193,18 +193,18 @@ NativeConnection.prototype.createClient = async function createClient(conn, uri, options.config.autoIndex : options.autoIndex; if (autoIndex != null) { - conn.config.autoIndex = autoIndex !== false; + this.config.autoIndex = autoIndex !== false; delete options.config; delete options.autoIndex; } if ('autoCreate' in options) { - conn.config.autoCreate = !!options.autoCreate; + this.config.autoCreate = !!options.autoCreate; delete options.autoCreate; } if ('sanitizeFilter' in options) { - conn.config.sanitizeFilter = options.sanitizeFilter; + this.config.sanitizeFilter = options.sanitizeFilter; delete options.sanitizeFilter; } @@ -214,24 +214,24 @@ NativeConnection.prototype.createClient = async function createClient(conn, uri, options.auth.username = options.user; options.auth.password = options.pass; - conn.user = options.user; - conn.pass = options.pass; + this.user = options.user; + this.pass = options.pass; } delete options.user; delete options.pass; if (options.bufferCommands != null) { - conn.config.bufferCommands = options.bufferCommands; + this.config.bufferCommands = options.bufferCommands; delete options.bufferCommands; } } else { options = {}; } - conn._connectionOptions = options; + this._connectionOptions = options; const dbName = options.dbName; if (dbName != null) { - conn.$dbName = dbName; + this.$dbName = dbName; } delete options.dbName; @@ -242,27 +242,27 @@ NativeConnection.prototype.createClient = async function createClient(conn, uri, }; } - conn.readyState = STATES.connecting; - conn._connectionString = uri; + this.readyState = STATES.connecting; + this._connectionString = uri; let client; try { client = new mongodb.MongoClient(uri, options); } catch (error) { - conn.readyState = STATES.disconnected; + this.readyState = STATES.disconnected; throw error; } - conn.client = client; + this.client = client; client.setMaxListeners(0); await client.connect(); - _setClient(conn, client, options, dbName); + _setClient(this, client, options, dbName); - for (const db of conn.otherDbs) { + for (const db of this.otherDbs) { _setClient(db, client, {}, db.name); } - return conn; + return this; }; /*! From b518fc496c51481c287a6477cb6aa7581c0b1787 Mon Sep 17 00:00:00 2001 From: hasezoey Date: Mon, 26 Jun 2023 14:30:44 +0200 Subject: [PATCH 16/34] feat(model): create: add option "immediateError" fixes #1731 --- lib/model.js | 50 ++++++++++++++++++++++++++++----------- test/model.create.test.js | 46 +++++++++++++++++++++++++++++++++++ types/models.d.ts | 6 ++++- 3 files changed, 87 insertions(+), 15 deletions(-) diff --git a/lib/model.js b/lib/model.js index 71a15f83ded..4420eaa70ec 100644 --- a/lib/model.js +++ b/lib/model.js @@ -2802,6 +2802,7 @@ Model.findByIdAndRemove = function(id, options) { * * @param {Array|Object} docs Documents to insert, as a spread or array * @param {Object} [options] Options passed down to `save()`. To specify `options`, `docs` **must** be an array, not a spread. See [Model.save](https://mongoosejs.com/docs/api/model.html#Model.prototype.save()) for available options. + * @param {Boolean} [options.immediateError] If a Error occurs, throw the error immediately, instead of collecting in a result, default: true (backwards compatability) * @return {Promise} * @api public */ @@ -2858,27 +2859,41 @@ Model.create = async function create(doc, options) { return Array.isArray(doc) ? [] : null; } let res = []; + const immediateError = typeof options.immediateError === 'boolean' ? options.immediateError : true; + + delete options.immediateError; // dont pass on the option to "$save" + if (options.ordered) { for (let i = 0; i < args.length; i++) { - const doc = args[i]; - const Model = this.discriminators && doc[discriminatorKey] != null ? - this.discriminators[doc[discriminatorKey]] || getDiscriminatorByValue(this.discriminators, doc[discriminatorKey]) : - this; - if (Model == null) { - throw new MongooseError(`Discriminator "${doc[discriminatorKey]}" not ` + + try { + const doc = args[i]; + const Model = this.discriminators && doc[discriminatorKey] != null ? + this.discriminators[doc[discriminatorKey]] || getDiscriminatorByValue(this.discriminators, doc[discriminatorKey]) : + this; + if (Model == null) { + throw new MongooseError(`Discriminator "${doc[discriminatorKey]}" not ` + `found for model "${this.modelName}"`); - } - let toSave = doc; - if (!(toSave instanceof Model)) { - toSave = new Model(toSave); - } + } + let toSave = doc; + if (!(toSave instanceof Model)) { + toSave = new Model(toSave); + } - await toSave.$save(options); - res.push(toSave); + await toSave.$save(options); + res.push(toSave); + } catch (err) { + if (!immediateError) { + res.push(err); + } else { + throw err; + } + } } return res; } else { - res = await Promise.all(args.map(async doc => { + // ".bind(Promise)" is required, otherwise results in "TypeError: Promise.allSettled called on non-object" + const promiseType = !immediateError ? Promise.allSettled.bind(Promise) : Promise.all.bind(Promise); + let p = promiseType(args.map(async doc => { const Model = this.discriminators && doc[discriminatorKey] != null ? this.discriminators[doc[discriminatorKey]] || getDiscriminatorByValue(this.discriminators, doc[discriminatorKey]) : this; @@ -2896,6 +2911,13 @@ Model.create = async function create(doc, options) { return toSave; })); + + // chain the mapper, only if "allSettled" is used + if (!immediateError) { + p = p.then(presult => presult.map(v => v.status === 'fulfilled' ? v.value : v.reason)); + } + + res = await p; } diff --git a/test/model.create.test.js b/test/model.create.test.js index 20036fb5397..f2fc65c463b 100644 --- a/test/model.create.test.js +++ b/test/model.create.test.js @@ -197,6 +197,52 @@ describe('model', function() { const docs = await Test.find(); assert.equal(docs.length, 5); }); + + it('should return the first error immediately if "immediateError" is not explicitly set (ordered)', async function() { + const testSchema = new Schema({ name: { type: String, required: true } }); + + const TestModel = db.model('gh1731-1', testSchema); + + const res = await TestModel.create([{ name: 'test' }, {}, { name: 'another' }], { ordered: true }).then(null).catch(err => err); + + assert.ok(res instanceof mongoose.Error.ValidationError); + }); + + it('should not return errors immediately if "immediateError" is "false" (ordered)', async function() { + const testSchema = new Schema({ name: { type: String, required: true } }); + + const TestModel = db.model('gh1731-2', testSchema); + + const res = await TestModel.create([{ name: 'test' }, {}, { name: 'another' }], { ordered: true, immediateError: false }); + + assert.equal(res.length, 3); + assert.ok(res[0] instanceof mongoose.Document); + assert.ok(res[1] instanceof mongoose.Error.ValidationError); + assert.ok(res[2] instanceof mongoose.Document); + }); + }); + + it('should return the first error immediately if "immediateError" is not explicitly set', async function() { + const testSchema = new Schema({ name: { type: String, required: true } }); + + const TestModel = db.model('gh1731-3', testSchema); + + const res = await TestModel.create([{ name: 'test' }, {}, { name: 'another' }], {}).then(null).catch(err => err); + + assert.ok(res instanceof mongoose.Error.ValidationError); + }); + + it('should not return errors immediately if "immediateError" is "false"', async function() { + const testSchema = new Schema({ name: { type: String, required: true } }); + + const TestModel = db.model('gh1731-4', testSchema); + + const res = await TestModel.create([{ name: 'test' }, {}, { name: 'another' }], { immediateError: false }); + + assert.equal(res.length, 3); + assert.ok(res[0] instanceof mongoose.Document); + assert.ok(res[1] instanceof mongoose.Error.ValidationError); + assert.ok(res[2] instanceof mongoose.Document); }); }); }); diff --git a/types/models.d.ts b/types/models.d.ts index ca844fcb94e..3864eac70e4 100644 --- a/types/models.d.ts +++ b/types/models.d.ts @@ -132,6 +132,10 @@ declare module 'mongoose' { wtimeout?: number; } + interface CreateOptions extends SaveOptions { + immediateError?: boolean; + } + interface RemoveOptions extends SessionOption, Omit {} const Model: Model; @@ -217,7 +221,7 @@ declare module 'mongoose' { >; /** Creates a new document or documents */ - create>(docs: Array, options?: SaveOptions): Promise; + create>(docs: Array, options?: CreateOptions): Promise; create>(doc: DocContents | TRawDocType): Promise; create>(...docs: Array): Promise; From 1bb735fe84a7f3d232408a7f608f911af469121f Mon Sep 17 00:00:00 2001 From: hasezoey Date: Mon, 26 Jun 2023 14:31:26 +0200 Subject: [PATCH 17/34] style(model): move option "ordered" from "save" to "create" --- lib/model.js | 2 +- types/models.d.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/model.js b/lib/model.js index 4420eaa70ec..0124d2425e2 100644 --- a/lib/model.js +++ b/lib/model.js @@ -479,7 +479,6 @@ function generateVersionError(doc, modifiedPaths) { * newProduct === product; // true * * @param {Object} [options] options optional options - * @param {Boolean} [options.ordered] saves the docs in series rather than parallel. * @param {Session} [options.session=null] the [session](https://www.mongodb.com/docs/manual/reference/server-sessions/) associated with this save operation. If not specified, defaults to the [document's associated session](https://mongoosejs.com/docs/api/document.html#Document.prototype.session()). * @param {Object} [options.safe] (DEPRECATED) overrides [schema's safe option](https://mongoosejs.com/docs/guide.html#safe). Use the `w` option instead. * @param {Boolean} [options.validateBeforeSave] set to false to save without validating. @@ -2802,6 +2801,7 @@ Model.findByIdAndRemove = function(id, options) { * * @param {Array|Object} docs Documents to insert, as a spread or array * @param {Object} [options] Options passed down to `save()`. To specify `options`, `docs` **must** be an array, not a spread. See [Model.save](https://mongoosejs.com/docs/api/model.html#Model.prototype.save()) for available options. + * @param {Boolean} [options.ordered] saves the docs in series rather than parallel. * @param {Boolean} [options.immediateError] If a Error occurs, throw the error immediately, instead of collecting in a result, default: true (backwards compatability) * @return {Promise} * @api public diff --git a/types/models.d.ts b/types/models.d.ts index 3864eac70e4..52c462f94e8 100644 --- a/types/models.d.ts +++ b/types/models.d.ts @@ -123,7 +123,6 @@ declare module 'mongoose' { SessionOption { checkKeys?: boolean; j?: boolean; - ordered?: boolean; safe?: boolean | WriteConcern; timestamps?: boolean | QueryTimestampsConfig; validateBeforeSave?: boolean; @@ -133,6 +132,7 @@ declare module 'mongoose' { } interface CreateOptions extends SaveOptions { + ordered?: boolean; immediateError?: boolean; } From a7780a2c50e7913b53a8ca42471250e27028f08b Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 26 Jun 2023 17:02:47 -0400 Subject: [PATCH 18/34] Update schematype.test.js --- test/schematype.test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/schematype.test.js b/test/schematype.test.js index a2660e9f517..3812b33616f 100644 --- a/test/schematype.test.js +++ b/test/schematype.test.js @@ -4,7 +4,9 @@ * Module dependencies. */ -const mongoose = require('./common').mongoose; +const start = require('./common'); + +const mongoose = start.mongoose; const assert = require('assert'); @@ -213,7 +215,7 @@ describe('schematype', function() { }); it('should allow setting values to a given property gh-13510', async function() { const m = new mongoose.Mongoose(); - await m.connect('mongodb://127.0.0.1:27017/gh13510'); + await m.connect(start.uri); m.SchemaTypes.Date.setters.push(v => typeof v === 'string' && /^\d{8}$/.test(v) ? new Date(v.slice(0, 4), +v.slice(4, 6) - 1, v.slice(6, 8)) : v); const testSchema = new m.Schema({ myDate: Date From 1fb95ac2c5055ceb524484d2004d0684ebe7cd2a Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 28 Jun 2023 11:12:08 -0400 Subject: [PATCH 19/34] test: fix deno tests --- test/document.test.js | 6 +++++- test/schematype.test.js | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/test/document.test.js b/test/document.test.js index 1e6c9ef7afd..f52caaab26c 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -7615,7 +7615,11 @@ describe('document', function() { schema.path('createdAt').immutable(true); assert.ok(schema.path('createdAt').$immutable); - assert.equal(schema.path('createdAt').setters.length, 1); + assert.equal( + schema.path('createdAt').setters.length, + 1, + schema.path('createdAt').setters.map(setter => setter.toString()) + ); schema.path('createdAt').immutable(false); assert.ok(!schema.path('createdAt').$immutable); diff --git a/test/schematype.test.js b/test/schematype.test.js index 3812b33616f..13eb8c5dce4 100644 --- a/test/schematype.test.js +++ b/test/schematype.test.js @@ -228,6 +228,10 @@ describe('schematype', function() { await m.connections[0].close(); assert(doc.myDate instanceof Date); }); + + after(() => { + mongoose.SchemaTypes.Date.setters = []; + }); }); const typesToTest = Object.values(mongoose.SchemaTypes). From 7d0972ac0e924ae5e1553fa124b16f34b1c9bfda Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 28 Jun 2023 13:03:45 -0400 Subject: [PATCH 20/34] test: fix deno tests --- test/query.toconstructor.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/query.toconstructor.test.js b/test/query.toconstructor.test.js index c97eb8cbaba..1de9f78f152 100644 --- a/test/query.toconstructor.test.js +++ b/test/query.toconstructor.test.js @@ -162,10 +162,11 @@ describe('Query:', function() { }); const Test = db.model('Test', schema); + await Test.init(); + await Test.deleteMany({}); const test = new Test({ name: 'Romero' }); const Q = Test.findOne({}).toConstructor(); - await test.save(); const doc = await Q(); assert.strictEqual(doc.name, 'Romero'); From 87284f402460919fae58ee7889382b08f8e1eb83 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 4 Jul 2023 16:50:40 -0400 Subject: [PATCH 21/34] feat(connection): add `Connection.prototype.removeDb()` for removing a related connection Fix #11821 --- lib/connection.js | 23 +++++++++++ lib/drivers/node-mongodb-native/connection.js | 38 +++++++++++++++++++ test/connection.test.js | 30 +++++++++++++++ 3 files changed, 91 insertions(+) diff --git a/lib/connection.js b/lib/connection.js index c68f20e7a48..e77e0b9b757 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -1418,6 +1418,29 @@ Connection.prototype.syncIndexes = async function syncIndexes(options = {}) { * @api public */ +/** + * Removes the database connection with the given name created with with `useDb()`. + * + * Throws an error if the database connection was not found. + * + * #### Example: + * + * // Connect to `initialdb` first + * const conn = await mongoose.createConnection('mongodb://127.0.0.1:27017/initialdb').asPromise(); + * + * // Creates an un-cached connection to `mydb` + * const db = conn.useDb('mydb'); + * + * // Closes `db`, and removes `db` from `conn.relatedDbs` and `conn.otherDbs` + * await conn.removeDb('mydb'); + * + * @method removeDb + * @memberOf Connection + * @param {String} name The database name + * @return {Connection} this + * @api public + */ + /*! * Module exports. */ diff --git a/lib/drivers/node-mongodb-native/connection.js b/lib/drivers/node-mongodb-native/connection.js index 417085be12e..42a1f25333b 100644 --- a/lib/drivers/node-mongodb-native/connection.js +++ b/lib/drivers/node-mongodb-native/connection.js @@ -126,6 +126,44 @@ NativeConnection.prototype.useDb = function(name, options) { return newConn; }; +/** + * Removes the database connection with the given name created with with `useDb()`. + * + * Throws an error if the database connection was not found. + * + * #### Example: + * + * // Connect to `initialdb` first + * const conn = await mongoose.createConnection('mongodb://127.0.0.1:27017/initialdb').asPromise(); + * + * // Creates an un-cached connection to `mydb` + * const db = conn.useDb('mydb'); + * + * // Closes `db`, and removes `db` from `conn.relatedDbs` and `conn.otherDbs` + * await conn.removeDb('mydb'); + * + * @method removeDb + * @memberOf Connection + * @param {String} name The database name + * @return {Connection} this + */ + +NativeConnection.prototype.removeDb = function removeDb(name) { + const dbs = this.otherDbs.filter(db => db.name === name); + if (!dbs.length) { + throw new MongooseError(`No connections to database "${name}" found`); + } + + for (const db of dbs) { + db._closeCalled = true; + db._destroyCalled = true; + db._readyState = STATES.disconnected; + db.$wasForceClosed = true; + } + delete this.relatedDbs[name]; + this.otherDbs = this.otherDbs.filter(db => db.name !== name); +}; + /** * Closes the connection * diff --git a/test/connection.test.js b/test/connection.test.js index db79263a74d..2365f5c3338 100644 --- a/test/connection.test.js +++ b/test/connection.test.js @@ -6,6 +6,7 @@ const start = require('./common'); +const STATES = require('../lib/connectionstate'); const Q = require('q'); const assert = require('assert'); const mongodb = require('mongodb'); @@ -746,6 +747,35 @@ describe('connections:', function() { assert.strictEqual(db2, db3); return db.close(); }); + + it('supports removing db (gh-11821)', async function() { + const db = await mongoose.createConnection(start.uri).asPromise(); + + const schema = mongoose.Schema({ name: String }, { autoCreate: false, autoIndex: false }); + const Test = db.model('Test', schema); + await Test.deleteMany({}); + await Test.create({ name: 'gh-11821' }); + + const db2 = db.useDb(start.databases[1]); + const Test2 = db2.model('Test', schema); + + await Test2.deleteMany({}); + let doc = await Test2.findOne(); + assert.equal(doc, null); + + db.removeDb(start.databases[1]); + assert.equal(db2.readyState, STATES.disconnected); + assert.equal(db.readyState, STATES.connected); + await assert.rejects( + () => Test2.findOne(), + /Connection was force closed/ + ); + + doc = await Test.findOne(); + assert.equal(doc.name, 'gh-11821'); + + await db.close(); + }); }); describe('shouldAuthenticate()', function() { From 84e6e0d4c6c2793cc17121685acfd60819676f62 Mon Sep 17 00:00:00 2001 From: hasezoey Date: Wed, 5 Jul 2023 12:42:35 +0200 Subject: [PATCH 22/34] fix(model): rename option "immediateError" to "aggregateErrors" --- lib/model.js | 6 +++--- types/models.d.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/model.js b/lib/model.js index 0124d2425e2..e72cfdebbed 100644 --- a/lib/model.js +++ b/lib/model.js @@ -2802,7 +2802,7 @@ Model.findByIdAndRemove = function(id, options) { * @param {Array|Object} docs Documents to insert, as a spread or array * @param {Object} [options] Options passed down to `save()`. To specify `options`, `docs` **must** be an array, not a spread. See [Model.save](https://mongoosejs.com/docs/api/model.html#Model.prototype.save()) for available options. * @param {Boolean} [options.ordered] saves the docs in series rather than parallel. - * @param {Boolean} [options.immediateError] If a Error occurs, throw the error immediately, instead of collecting in a result, default: true (backwards compatability) + * @param {Boolean} [options.aggregateErrors] Aggregate Errors instead of throwing the first one that occurs. Default: false * @return {Promise} * @api public */ @@ -2859,9 +2859,9 @@ Model.create = async function create(doc, options) { return Array.isArray(doc) ? [] : null; } let res = []; - const immediateError = typeof options.immediateError === 'boolean' ? options.immediateError : true; + const immediateError = typeof options.aggregateErrors === 'boolean' ? !options.aggregateErrors : true; - delete options.immediateError; // dont pass on the option to "$save" + delete options.aggregateErrors; // dont pass on the option to "$save" if (options.ordered) { for (let i = 0; i < args.length; i++) { diff --git a/types/models.d.ts b/types/models.d.ts index 52c462f94e8..fc7372e0c0c 100644 --- a/types/models.d.ts +++ b/types/models.d.ts @@ -133,7 +133,7 @@ declare module 'mongoose' { interface CreateOptions extends SaveOptions { ordered?: boolean; - immediateError?: boolean; + aggregateErrors?: boolean; } interface RemoveOptions extends SessionOption, Omit {} From 3f442865836297295ed84473fb3765dd8d400bf0 Mon Sep 17 00:00:00 2001 From: hasezoey Date: Wed, 5 Jul 2023 12:44:35 +0200 Subject: [PATCH 23/34] fix(types): add overload for "create" with "aggregateErrors: true" --- test/types/create.test.ts | 9 ++++++++- types/models.d.ts | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/test/types/create.test.ts b/test/types/create.test.ts index 3686bf077f8..c0a7b0e5630 100644 --- a/test/types/create.test.ts +++ b/test/types/create.test.ts @@ -1,4 +1,4 @@ -import { Schema, model, Types, CallbackError } from 'mongoose'; +import { Schema, model, Types, HydratedDocument } from 'mongoose'; import { expectError, expectType } from 'tsd'; const schema = new Schema({ name: { type: 'String' } }); @@ -118,3 +118,10 @@ Test.insertMany({ _id: new Types.ObjectId('000000000000000000000000'), name: 'te (await Test.create([{ name: 'test' }]))[0]; (await Test.create({ name: 'test' }))._id; })(); + +async function createWithAggregateErrors() { + expectType<(HydratedDocument)[]>(await Test.create([{}])); + expectType<(HydratedDocument | Error)[]>(await Test.create([{}], { aggregateErrors: true })); +} + +createWithAggregateErrors(); diff --git a/types/models.d.ts b/types/models.d.ts index fc7372e0c0c..bab0683c98b 100644 --- a/types/models.d.ts +++ b/types/models.d.ts @@ -221,6 +221,7 @@ declare module 'mongoose' { >; /** Creates a new document or documents */ + create>(docs: Array, options: CreateOptions & { aggregateErrors: true }): Promise<(THydratedDocumentType | Error)[]>; create>(docs: Array, options?: CreateOptions): Promise; create>(doc: DocContents | TRawDocType): Promise; create>(...docs: Array): Promise; From f4d41057ceb306a1459fbf7b5f43859213633070 Mon Sep 17 00:00:00 2001 From: hasezoey Date: Wed, 5 Jul 2023 12:48:40 +0200 Subject: [PATCH 24/34] test(model.create): update for "aggregateErrors" rename --- test/model.create.test.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/model.create.test.js b/test/model.create.test.js index f2fc65c463b..3b2182e8259 100644 --- a/test/model.create.test.js +++ b/test/model.create.test.js @@ -198,7 +198,7 @@ describe('model', function() { assert.equal(docs.length, 5); }); - it('should return the first error immediately if "immediateError" is not explicitly set (ordered)', async function() { + it('should return the first error immediately if "aggregateErrors" is not explicitly set (ordered)', async function() { const testSchema = new Schema({ name: { type: String, required: true } }); const TestModel = db.model('gh1731-1', testSchema); @@ -208,12 +208,12 @@ describe('model', function() { assert.ok(res instanceof mongoose.Error.ValidationError); }); - it('should not return errors immediately if "immediateError" is "false" (ordered)', async function() { + it('should not return errors immediately if "aggregateErrors" is "true" (ordered)', async function() { const testSchema = new Schema({ name: { type: String, required: true } }); const TestModel = db.model('gh1731-2', testSchema); - const res = await TestModel.create([{ name: 'test' }, {}, { name: 'another' }], { ordered: true, immediateError: false }); + const res = await TestModel.create([{ name: 'test' }, {}, { name: 'another' }], { ordered: true, aggregateErrors: true }); assert.equal(res.length, 3); assert.ok(res[0] instanceof mongoose.Document); @@ -222,7 +222,7 @@ describe('model', function() { }); }); - it('should return the first error immediately if "immediateError" is not explicitly set', async function() { + it('should return the first error immediately if "aggregateErrors" is not explicitly set', async function() { const testSchema = new Schema({ name: { type: String, required: true } }); const TestModel = db.model('gh1731-3', testSchema); @@ -232,12 +232,12 @@ describe('model', function() { assert.ok(res instanceof mongoose.Error.ValidationError); }); - it('should not return errors immediately if "immediateError" is "false"', async function() { + it('should not return errors immediately if "aggregateErrors" is "true"', async function() { const testSchema = new Schema({ name: { type: String, required: true } }); const TestModel = db.model('gh1731-4', testSchema); - const res = await TestModel.create([{ name: 'test' }, {}, { name: 'another' }], { immediateError: false }); + const res = await TestModel.create([{ name: 'test' }, {}, { name: 'another' }], { aggregateErrors: true }); assert.equal(res.length, 3); assert.ok(res[0] instanceof mongoose.Document); From 6ac57e146a0071c00a88af7f4fb762ad756275b0 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 5 Jul 2023 07:21:19 -0400 Subject: [PATCH 25/34] Update lib/drivers/node-mongodb-native/connection.js Co-authored-by: hasezoey --- lib/drivers/node-mongodb-native/connection.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/drivers/node-mongodb-native/connection.js b/lib/drivers/node-mongodb-native/connection.js index 42a1f25333b..0e6a2c29836 100644 --- a/lib/drivers/node-mongodb-native/connection.js +++ b/lib/drivers/node-mongodb-native/connection.js @@ -127,7 +127,7 @@ NativeConnection.prototype.useDb = function(name, options) { }; /** - * Removes the database connection with the given name created with with `useDb()`. + * Removes the database connection with the given name created with `useDb()`. * * Throws an error if the database connection was not found. * From 0880a2e485fdfb69be2dfe0531ae4af030882cde Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Wed, 5 Jul 2023 11:23:38 -0400 Subject: [PATCH 26/34] `includeResultMetadata` option --- lib/query.js | 6 +++--- test/model.findOneAndUpdate.test.js | 13 +++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/query.js b/lib/query.js index 2e32853f117..edf8264b71d 100644 --- a/lib/query.js +++ b/lib/query.js @@ -2435,7 +2435,7 @@ Query.prototype.collation = function(value) { */ Query.prototype._completeOne = function(doc, res, callback) { - if (!doc && !this.options.rawResult) { + if (!doc && !this.options.rawResult && !this.options.includeResultMetadata) { return callback(null, null); } @@ -3099,7 +3099,7 @@ Query.prototype._deleteMany = async function _deleteMany() { */ function completeOne(model, doc, res, options, fields, userProvidedFields, pop, callback) { - if (options.rawResult && doc == null) { + if ((options.rawResult || options.includeResultMetadata) && doc == null) { _init(null); return null; } @@ -3112,7 +3112,7 @@ function completeOne(model, doc, res, options, fields, userProvidedFields, pop, } - if (options.rawResult) { + if (options.rawResult || options.includeResultMetadata) { if (doc && casted) { if (options.session != null) { casted.$session(options.session); diff --git a/test/model.findOneAndUpdate.test.js b/test/model.findOneAndUpdate.test.js index 8ab0c67244b..cd8f53a53c3 100644 --- a/test/model.findOneAndUpdate.test.js +++ b/test/model.findOneAndUpdate.test.js @@ -2143,4 +2143,17 @@ describe('model: findOneAndUpdate:', function() { assert.equal(doc.info['second.name'], 'Quiz'); assert.equal(doc.info2['second.name'], 'Quiz'); }); + it('supports the `includeResultMetadata` option (gh-13539)', async function() { + const testSchema = new mongoose.Schema({ + name: String + }); + const Test = db.model('Test', testSchema); + await Test.create({ + name: 'Test' + }); + const doc = await Test.findOneAndUpdate({ name: 'Test' }, { name: 'Test Testerson' }, { new: true, upsert: true, includeResultMetadata: false }); + assert.equal(doc.ok, undefined); + const data = await Test.findOneAndUpdate({ name: 'Test Testerson' }, { name: 'Test' }, { new: true, upsert: true, includeResultMetadata: true }); + assert(data.ok); + }); }); From cec2ff46bcbf404191d880765937517e4f3c762d Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 6 Jul 2023 10:39:49 -0400 Subject: [PATCH 27/34] Update model.findOneAndUpdate.test.js --- test/model.findOneAndUpdate.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/model.findOneAndUpdate.test.js b/test/model.findOneAndUpdate.test.js index cd8f53a53c3..779437ee3df 100644 --- a/test/model.findOneAndUpdate.test.js +++ b/test/model.findOneAndUpdate.test.js @@ -2153,7 +2153,9 @@ describe('model: findOneAndUpdate:', function() { }); const doc = await Test.findOneAndUpdate({ name: 'Test' }, { name: 'Test Testerson' }, { new: true, upsert: true, includeResultMetadata: false }); assert.equal(doc.ok, undefined); + assert.equal(doc.name, 'Test Testerson'); const data = await Test.findOneAndUpdate({ name: 'Test Testerson' }, { name: 'Test' }, { new: true, upsert: true, includeResultMetadata: true }); assert(data.ok); + assert.equal(data.value.name, 'Test'); }); }); From d0ade4e5ee8f504a3f65c9e48be39168925cb0d2 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 9 Jul 2023 15:29:18 -0400 Subject: [PATCH 28/34] feat: upgrade to MongoDB Node.js driver 5.7.0 --- docs/deprecations.md | 110 ++++----------------------- lib/query.js | 33 +++++++- package.json | 4 +- test/model.findOneAndDelete.test.js | 38 +++++++++ test/model.findOneAndReplace.test.js | 42 ++++++++++ test/model.findOneAndUpdate.test.js | 30 +++++++- 6 files changed, 156 insertions(+), 101 deletions(-) diff --git a/docs/deprecations.md b/docs/deprecations.md index f96a7791b89..08003401a0b 100644 --- a/docs/deprecations.md +++ b/docs/deprecations.md @@ -9,108 +9,30 @@ cause any problems for your application. Please [report any issues on GitHub](ht To fix all deprecation warnings, follow the below steps: -* Replace `update()` with `updateOne()`, `updateMany()`, or `replaceOne()` -* Replace `remove()` with `deleteOne()` or `deleteMany()`. -* Replace `count()` with `countDocuments()`, unless you want to count how many documents are in the whole collection (no filter). In the latter case, use `estimatedDocumentCount()`. +* Replace `rawResult: true` with `includeResultMetadata: false` in `findOneAndUpdate()`, `findOneAndReplace()`, `findOneAndDelete()` calls. Read below for more a more detailed description of each deprecation warning. -

remove()

+

rawResult

-The MongoDB driver's [`remove()` function](http://mongodb.github.io/node-mongodb-native/3.1/api/Collection.html#remove) is deprecated in favor of `deleteOne()` and `deleteMany()`. This is to comply with -the [MongoDB CRUD specification](https://github.com/mongodb/specifications/blob/master/source/crud/crud.rst), -which aims to provide a consistent API for CRUD operations across all MongoDB -drivers. - -```txt -DeprecationWarning: collection.remove is deprecated. Use deleteOne, -deleteMany, or bulkWrite instead. -``` - -To remove this deprecation warning, replace any usage of `remove()` with -`deleteMany()`, *unless* you specify the [`single` option to `remove()`](api/model.html#model_Model-remove). The `single` -option limited `remove()` to deleting at most one document, so you should -replace `remove(filter, { single: true })` with `deleteOne(filter)`. +As of Mongoose 7.4.0, the `rawResult` option to `findOneAndUpdate()` is deprecated. +You should instead use the `includeResultMetadata` option, which the MongoDB Node.js driver's new option that replaces `rawResult`. ```javascript // Replace this: -MyModel.remove({ foo: 'bar' }); -// With this: -MyModel.deleteMany({ foo: 'bar' }); - -// Replace this: -MyModel.remove({ answer: 42 }, { single: true }); -// With this: -MyModel.deleteOne({ answer: 42 }); -``` +const doc = await Test.findOneAndUpdate( + { name: 'Test' }, + { name: 'Test Testerson' }, + { rawResult: true } +); -

update()

- -Like `remove()`, the [`update()` function](api/model.html#model_Model-update) is deprecated in favor -of the more explicit [`updateOne()`](api/model.html#model_Model-updateOne), [`updateMany()`](api/model.html#model_Model-updateMany), and [`replaceOne()`](api/model.html#model_Model-replaceOne) functions. You should replace -`update()` with `updateOne()`, unless you use the [`multi` or `overwrite` options](api/model.html#model_Model-update). - -```txt -collection.update is deprecated. Use updateOne, updateMany, or bulkWrite -instead. -``` - -```javascript -// Replace this: -MyModel.update({ foo: 'bar' }, { answer: 42 }); // With this: -MyModel.updateOne({ foo: 'bar' }, { answer: 42 }); - -// If you use `overwrite: true`, you should use `replaceOne()` instead: -MyModel.update(filter, update, { overwrite: true }); -// Replace with this: -MyModel.replaceOne(filter, update); - -// If you use `multi: true`, you should use `updateMany()` instead: -MyModel.update(filter, update, { multi: true }); -// Replace with this: -MyModel.updateMany(filter, update); +const doc = await Test.findOneAndUpdate( + { name: 'Test' }, + { name: 'Test Testerson' }, + { includeResultMetadata: false } +); ``` -

count()

- -The MongoDB server has deprecated the `count()` function in favor of two -separate functions, [`countDocuments()`](api/query.html#Query.prototype.countDocuments()) and -[`estimatedDocumentCount()`](api/query.html#Query.prototype.estimatedDocumentCount()). - -```txt -DeprecationWarning: collection.count is deprecated, and will be removed in a future version. Use collection.countDocuments or collection.estimatedDocumentCount instead -``` - -The difference between the two is `countDocuments()` can accept a filter -parameter like [`find()`](api/query.html#Query.prototype.find()). The `estimatedDocumentCount()` -function is faster, but can only tell you the total number of documents in -a collection. You cannot pass a `filter` to `estimatedDocumentCount()`. - -To migrate, replace `count()` with `countDocuments()` *unless* you do not -pass any arguments to `count()`. If you use `count()` to count all documents -in a collection as opposed to counting documents that match a query, use -`estimatedDocumentCount()` instead of `countDocuments()`. - -```javascript -// Replace this: -MyModel.count({ answer: 42 }); -// With this: -MyModel.countDocuments({ answer: 42 }); - -// If you're counting all documents in the collection, use -// `estimatedDocumentCount()` instead. -MyModel.count(); -// Replace with: -MyModel.estimatedDocumentCount(); - -// Replace this: -MyModel.find({ answer: 42 }).count().exec(); -// With this: -MyModel.find({ answer: 42 }).countDocuments().exec(); - -// Replace this: -MyModel.find().count().exec(); -// With this, since there's no filter -MyModel.find().estimatedDocumentCount().exec(); -``` +The `rawResult` option only affects Mongoose; the MongoDB Node.js driver still returns the full result metadata, Mongoose just parses out the raw document. +The `includeResultMetadata` option also tells the MongoDB Node.js driver to only return the document, not the full `ModifyResult` object. diff --git a/lib/query.js b/lib/query.js index 4f21f40f821..ac2eab0f137 100644 --- a/lib/query.js +++ b/lib/query.js @@ -1636,6 +1636,10 @@ Query.prototype.setOptions = function(options, overwrite) { delete options.translateAliases; } + if ('rawResult' in options) { + printRawResultDeprecationWarning(); + } + if (options.lean == null && this.schema && 'lean' in this.schema.options) { this._mongooseOptions.lean = this.schema.options.lean; } @@ -1670,6 +1674,15 @@ Query.prototype.setOptions = function(options, overwrite) { return this; }; +/*! + * ignore + */ + +const printRawResultDeprecationWarning = util.deprecate( + function printRawResultDeprecationWarning() {}, + 'The `rawResult` option for Mongoose queries is deprecated. Use `includeResultMetadata: false` as a replacement for `rawResult: true`.' +); + /** * Sets the [`explain` option](https://www.mongodb.com/docs/manual/reference/method/cursor.explain/), * which makes this query return detailed execution stats instead of the actual @@ -3287,6 +3300,10 @@ Query.prototype._findOneAndUpdate = async function _findOneAndUpdate() { applyGlobalMaxTimeMS(this.options, this.model); applyGlobalDiskUse(this.options, this.model); + if (this.options.rawResult && this.options.includeResultMetadata === false) { + throw new MongooseError('Cannot set `rawResult` option when `includeResultMetadata` is false'); + } + if ('strict' in this.options) { this._mongooseOptions.strict = this.options.strict; } @@ -3337,7 +3354,7 @@ Query.prototype._findOneAndUpdate = async function _findOneAndUpdate() { for (const fn of this._transforms) { res = fn(res); } - const doc = res.value; + const doc = options.includeResultMetadata === false ? res : res.value; return new Promise((resolve, reject) => { this._completeOne(doc, res, _wrapThunkCallback(this, (err, res) => { @@ -3478,6 +3495,11 @@ Query.prototype._findOneAndDelete = async function _findOneAndDelete() { throw this.error(); } + const includeResultMetadata = this.options.includeResultMetadata; + if (this.options.rawResult && includeResultMetadata === false) { + throw new MongooseError('Cannot set `rawResult` option when `includeResultMetadata` is false'); + } + const filter = this._conditions; const options = this._optionsForExec(this.model); this._applyTranslateAliases(options); @@ -3486,7 +3508,7 @@ Query.prototype._findOneAndDelete = async function _findOneAndDelete() { for (const fn of this._transforms) { res = fn(res); } - const doc = res.value; + const doc = includeResultMetadata === false ? res : res.value; return new Promise((resolve, reject) => { this._completeOne(doc, res, _wrapThunkCallback(this, (err, res) => { @@ -3613,6 +3635,11 @@ Query.prototype._findOneAndReplace = async function _findOneAndReplace() { this._applyTranslateAliases(options); convertNewToReturnDocument(options); + const includeResultMetadata = this.options.includeResultMetadata; + if (this.options.rawResult && includeResultMetadata === false) { + throw new MongooseError('Cannot set `rawResult` option when `includeResultMetadata` is false'); + } + const modelOpts = { skipId: true }; if ('strict' in this._mongooseOptions) { modelOpts.strict = this._mongooseOptions.strict; @@ -3643,7 +3670,7 @@ Query.prototype._findOneAndReplace = async function _findOneAndReplace() { res = fn(res); } - const doc = res.value; + const doc = includeResultMetadata === false ? res : res.value; return new Promise((resolve, reject) => { this._completeOne(doc, res, _wrapThunkCallback(this, (err, res) => { if (err) { diff --git a/package.json b/package.json index 8264ee20d0f..022e00aee9f 100644 --- a/package.json +++ b/package.json @@ -19,9 +19,9 @@ ], "license": "MIT", "dependencies": { - "bson": "^5.3.0", + "bson": "^5.4.0", "kareem": "2.5.1", - "mongodb": "5.6.0", + "mongodb": "5.7.0", "mpath": "0.9.0", "mquery": "5.0.0", "ms": "2.1.3", diff --git a/test/model.findOneAndDelete.test.js b/test/model.findOneAndDelete.test.js index 1653c3e8a3d..38986a9ca33 100644 --- a/test/model.findOneAndDelete.test.js +++ b/test/model.findOneAndDelete.test.js @@ -335,4 +335,42 @@ describe('model: findOneAndDelete:', function() { assert.equal(postCount, 1); }); }); + + it('supports the `includeResultMetadata` option (gh-13539)', async function() { + const testSchema = new mongoose.Schema({ + name: String + }); + const Test = db.model('Test', testSchema); + await Test.create({ name: 'Test' }); + const doc = await Test.findOneAndDelete( + { name: 'Test' }, + { includeResultMetadata: false } + ); + assert.equal(doc.ok, undefined); + assert.equal(doc.name, 'Test'); + + await Test.create({ name: 'Test' }); + let data = await Test.findOneAndDelete( + { name: 'Test' }, + { includeResultMetadata: true } + ); + assert(data.ok); + assert.equal(data.value.name, 'Test'); + + await Test.create({ name: 'Test' }); + data = await Test.findOneAndDelete( + { name: 'Test' }, + { includeResultMetadata: true, rawResult: true } + ); + assert(data.ok); + assert.equal(data.value.name, 'Test'); + + await assert.rejects( + () => Test.findOneAndDelete( + { name: 'Test' }, + { includeResultMetadata: false, rawResult: true } + ), + /Cannot set `rawResult` option when `includeResultMetadata` is false/ + ); + }); }); diff --git a/test/model.findOneAndReplace.test.js b/test/model.findOneAndReplace.test.js index d463a0b5924..5550f198915 100644 --- a/test/model.findOneAndReplace.test.js +++ b/test/model.findOneAndReplace.test.js @@ -451,4 +451,46 @@ describe('model: findOneAndReplace:', function() { assert.ok(!Object.keys(opts).includes('overwrite')); assert.ok(!Object.keys(opts).includes('timestamps')); }); + + it('supports the `includeResultMetadata` option (gh-13539)', async function() { + const testSchema = new mongoose.Schema({ + name: String + }); + const Test = db.model('Test', testSchema); + await Test.create({ + name: 'Test' + }); + const doc = await Test.findOneAndReplace( + { name: 'Test' }, + { name: 'Test Testerson' }, + { new: true, upsert: true, includeResultMetadata: false } + ); + assert.equal(doc.ok, undefined); + assert.equal(doc.name, 'Test Testerson'); + + let data = await Test.findOneAndReplace( + { name: 'Test Testerson' }, + { name: 'Test' }, + { new: true, upsert: true, includeResultMetadata: true } + ); + assert(data.ok); + assert.equal(data.value.name, 'Test'); + + data = await Test.findOneAndReplace( + { name: 'Test Testerson' }, + { name: 'Test' }, + { new: true, upsert: true, includeResultMetadata: true, rawResult: true } + ); + assert(data.ok); + assert.equal(data.value.name, 'Test'); + + await assert.rejects( + () => Test.findOneAndReplace( + { name: 'Test Testerson' }, + { name: 'Test' }, + { new: true, upsert: true, includeResultMetadata: false, rawResult: true } + ), + /Cannot set `rawResult` option when `includeResultMetadata` is false/ + ); + }); }); diff --git a/test/model.findOneAndUpdate.test.js b/test/model.findOneAndUpdate.test.js index 779437ee3df..18f9e5e8692 100644 --- a/test/model.findOneAndUpdate.test.js +++ b/test/model.findOneAndUpdate.test.js @@ -2151,11 +2151,37 @@ describe('model: findOneAndUpdate:', function() { await Test.create({ name: 'Test' }); - const doc = await Test.findOneAndUpdate({ name: 'Test' }, { name: 'Test Testerson' }, { new: true, upsert: true, includeResultMetadata: false }); + const doc = await Test.findOneAndUpdate( + { name: 'Test' }, + { name: 'Test Testerson' }, + { new: true, upsert: true, includeResultMetadata: false } + ); assert.equal(doc.ok, undefined); assert.equal(doc.name, 'Test Testerson'); - const data = await Test.findOneAndUpdate({ name: 'Test Testerson' }, { name: 'Test' }, { new: true, upsert: true, includeResultMetadata: true }); + + let data = await Test.findOneAndUpdate( + { name: 'Test Testerson' }, + { name: 'Test' }, + { new: true, upsert: true, includeResultMetadata: true } + ); + assert(data.ok); + assert.equal(data.value.name, 'Test'); + + data = await Test.findOneAndUpdate( + { name: 'Test Testerson' }, + { name: 'Test' }, + { new: true, upsert: true, includeResultMetadata: true, rawResult: true } + ); assert(data.ok); assert.equal(data.value.name, 'Test'); + + await assert.rejects( + () => Test.findOneAndUpdate( + { name: 'Test Testerson' }, + { name: 'Test' }, + { new: true, upsert: true, includeResultMetadata: false, rawResult: true } + ), + /Cannot set `rawResult` option when `includeResultMetadata` is false/ + ); }); }); From 9aba5fdc8f83f24dc24d89097c13dc359a8bef70 Mon Sep 17 00:00:00 2001 From: hasezoey Date: Mon, 10 Jul 2023 12:59:32 +0200 Subject: [PATCH 29/34] docs(guide): fix md lint --- docs/guide.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guide.md b/docs/guide.md index c5774d2f267..7de820affa0 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -526,7 +526,7 @@ Valid options: * [skipVersioning](#skipVersioning) * [timestamps](#timestamps) * [storeSubdocValidationError](#storeSubdocValidationError) -- [collectionOptions](#collectionOptions) +* [collectionOptions](#collectionOptions) * [methods](#methods) * [query](#query-helpers) From 57a5db5aea2148d082bae4b1d1037d0fd7371812 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 12 Jul 2023 18:10:44 -0400 Subject: [PATCH 30/34] feat: support generating custom cast error message with a function Fix #3162 --- lib/error/cast.js | 19 ++++++++++--------- lib/schematype.js | 20 +++++++++++++++++--- test/model.test.js | 6 +++--- test/schema.test.js | 19 +++++++++++++++++++ types/schematypes.d.ts | 6 +++++- 5 files changed, 54 insertions(+), 16 deletions(-) diff --git a/lib/error/cast.js b/lib/error/cast.js index c42e8216691..f7df49b8c7e 100644 --- a/lib/error/cast.js +++ b/lib/error/cast.js @@ -20,10 +20,9 @@ class CastError extends MongooseError { constructor(type, value, path, reason, schemaType) { // If no args, assume we'll `init()` later. if (arguments.length > 0) { - const stringValue = getStringValue(value); const valueType = getValueType(value); const messageFormat = getMessageFormat(schemaType); - const msg = formatMessage(null, type, stringValue, path, messageFormat, valueType, reason); + const msg = formatMessage(null, type, value, path, messageFormat, valueType, reason); super(msg); this.init(type, value, path, reason, schemaType); } else { @@ -77,7 +76,7 @@ class CastError extends MongooseError { */ setModel(model) { this.model = model; - this.message = formatMessage(model, this.kind, this.stringValue, this.path, + this.message = formatMessage(model, this.kind, this.value, this.path, this.messageFormat, this.valueType); } } @@ -111,10 +110,8 @@ function getValueType(value) { } function getMessageFormat(schemaType) { - const messageFormat = schemaType && - schemaType.options && - schemaType.options.cast || null; - if (typeof messageFormat === 'string') { + const messageFormat = schemaType && schemaType._castErrorMessage || null; + if (typeof messageFormat === 'string' || typeof messageFormat === 'function') { return messageFormat; } } @@ -123,8 +120,9 @@ function getMessageFormat(schemaType) { * ignore */ -function formatMessage(model, kind, stringValue, path, messageFormat, valueType, reason) { - if (messageFormat != null) { +function formatMessage(model, kind, value, path, messageFormat, valueType, reason) { + if (typeof messageFormat === 'string') { + const stringValue = getStringValue(value); let ret = messageFormat. replace('{KIND}', kind). replace('{VALUE}', stringValue). @@ -134,7 +132,10 @@ function formatMessage(model, kind, stringValue, path, messageFormat, valueType, } return ret; + } else if (typeof messageFormat === 'function') { + return messageFormat(value, path, model, kind); } else { + const stringValue = getStringValue(value); const valueTypeMsg = valueType ? ' (type ' + valueType + ')' : ''; let ret = 'Cast to ' + kind + ' failed for value ' + stringValue + valueTypeMsg + ' at path "' + path + '"'; diff --git a/lib/schematype.js b/lib/schematype.js index f5e6025510d..ef22979203b 100644 --- a/lib/schematype.js +++ b/lib/schematype.js @@ -82,7 +82,11 @@ function SchemaType(path, options, instance) { const keys = Object.keys(this.options); for (const prop of keys) { if (prop === 'cast') { - this.castFunction(this.options[prop]); + if (Array.isArray(this.options[prop])) { + this.castFunction.apply(this, this.options[prop]); + } else { + this.castFunction(this.options[prop]); + } continue; } if (utils.hasUserDefinedProperty(this.options, prop) && typeof this[prop] === 'function') { @@ -255,14 +259,24 @@ SchemaType.cast = function cast(caster) { * @api public */ -SchemaType.prototype.castFunction = function castFunction(caster) { +SchemaType.prototype.castFunction = function castFunction(caster, message) { if (arguments.length === 0) { return this._castFunction; } + if (caster === false) { caster = this.constructor._defaultCaster || (v => v); } - this._castFunction = caster; + if (typeof caster === 'string') { + this._castErrorMessage = caster; + return this._castFunction; + } + if (caster != null) { + this._castFunction = caster; + } + if (message != null) { + this._castErrorMessage = message; + } return this._castFunction; }; diff --git a/test/model.test.js b/test/model.test.js index 332cfffbfb4..0b2303a8ff1 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -208,8 +208,8 @@ describe('Model', function() { describe('defaults', function() { it('to a non-empty array', function() { const DefaultArraySchema = new Schema({ - arr: { type: Array, cast: String, default: ['a', 'b', 'c'] }, - single: { type: Array, cast: String, default: ['a'] } + arr: { type: Array, default: ['a', 'b', 'c'] }, + single: { type: Array, default: ['a'] } }); const DefaultArray = db.model('Test', DefaultArraySchema); const arr = new DefaultArray(); @@ -223,7 +223,7 @@ describe('Model', function() { it('empty', function() { const DefaultZeroCardArraySchema = new Schema({ - arr: { type: Array, cast: String, default: [] }, + arr: { type: Array, default: [] }, auto: [Number] }); const DefaultZeroCardArray = db.model('Test', DefaultZeroCardArraySchema); diff --git a/test/schema.test.js b/test/schema.test.js index 8752374b866..54145e1c0ea 100644 --- a/test/schema.test.js +++ b/test/schema.test.js @@ -2396,6 +2396,25 @@ describe('schema', function() { assert.ok(threw); }); + it('with function cast error format', function() { + const schema = Schema({ + num: { + type: Number, + cast: [null, value => `${value} isn't a number`] + } + }); + + let threw = false; + try { + schema.path('num').cast('horseradish'); + } catch (err) { + threw = true; + assert.equal(err.name, 'CastError'); + assert.equal(err.message, 'horseradish isn\'t a number'); + } + assert.ok(threw); + }); + it('with objectids', function() { const schema = Schema({ userId: { diff --git a/types/schematypes.d.ts b/types/schematypes.d.ts index cb1635a191a..b02f71a0744 100644 --- a/types/schematypes.d.ts +++ b/types/schematypes.d.ts @@ -63,7 +63,11 @@ declare module 'mongoose' { validate?: SchemaValidator | AnyArray>; /** Allows overriding casting logic for this individual path. If a string, the given string overwrites Mongoose's default cast error message. */ - cast?: string; + cast?: string | + boolean | + ((value: any) => T) | + [(value: any) => T, string] | + [((value: any) => T) | null, (value: any, path: string, model: Model, kind: string) => string]; /** * If true, attach a required validator to this path, which ensures this path From d6cf0a00ab8693b974d1143447a177c997db2e17 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 14 Jul 2023 15:56:07 -0400 Subject: [PATCH 31/34] docs: explain how to overwrite cast error messages in validation docs Re: #3162 --- docs/validation.md | 30 +++++++++++++++++------ lib/error/messages.js | 2 +- test/docs/validation.test.js | 47 ++++++++++++++++++++++++++++++++++++ types/schematypes.d.ts | 8 +++--- 4 files changed, 75 insertions(+), 12 deletions(-) diff --git a/docs/validation.md b/docs/validation.md index c0fc6d4aed1..5e7c87ff4d4 100644 --- a/docs/validation.md +++ b/docs/validation.md @@ -109,19 +109,35 @@ thrown. ## Cast Errors -Before running validators, Mongoose attempts to coerce values to the -correct type. This process is called *casting* the document. If -casting fails for a given path, the `error.errors` object will contain -a `CastError` object. +Before running validators, Mongoose attempts to coerce values to the correct type. This process is called *casting* the document. +If casting fails for a given path, the `error.errors` object will contain a `CastError` object. -Casting runs before validation, and validation does not run if casting -fails. That means your custom validators may assume `v` is `null`, -`undefined`, or an instance of the type specified in your schema. +Casting runs before validation, and validation does not run if casting fails. +That means your custom validators may assume `v` is `null`, `undefined`, or an instance of the type specified in your schema. ```acquit [require:Cast Errors] ``` +By default, Mongoose cast error messages look like `Cast to Number failed for value "pie" at path "numWheels"`. +You can overwrite Mongoose's default cast error message by the `cast` option on your SchemaType to a string as follows. + +```acquit +[require:Cast Error Message Overwrite] +``` + +Mongoose's cast error message templating supports the following parameters: + +- `{PATH}`: the path that failed to cast +- `{VALUE}`: a string representation of the value that failed to cast +- `{KIND}`: the type that Mongoose attempted to cast to, like `'String'` or `'Number'` + +You can also define a function that Mongoose will call to get the cast error message as follows. + +```acquit +[require:Cast Error Message Function Overwrite] +``` + ## Global SchemaType Validation In addition to defining custom validators on individual schema paths, you can also configure a custom validator to run on every instance of a given `SchemaType`. diff --git a/lib/error/messages.js b/lib/error/messages.js index b750d5d5ec2..a2db50a6fce 100644 --- a/lib/error/messages.js +++ b/lib/error/messages.js @@ -6,7 +6,7 @@ * const mongoose = require('mongoose'); * mongoose.Error.messages.String.enum = "Your custom message for {PATH}."; * - * As you might have noticed, error messages support basic templating + * Error messages support basic templating. Mongoose will replace the following strings with the corresponding value. * * - `{PATH}` is replaced with the invalid document path * - `{VALUE}` is replaced with the invalid value diff --git a/test/docs/validation.test.js b/test/docs/validation.test.js index fd4607b4925..a03cf494571 100644 --- a/test/docs/validation.test.js +++ b/test/docs/validation.test.js @@ -384,6 +384,53 @@ describe('validation docs', function() { // acquit:ignore:end }); + it('Cast Error Message Overwrite', function() { + const vehicleSchema = new mongoose.Schema({ + numWheels: { + type: Number, + cast: '{VALUE} is not a number' + } + }); + const Vehicle = db.model('Vehicle', vehicleSchema); + + const doc = new Vehicle({ numWheels: 'pie' }); + const err = doc.validateSync(); + + err.errors['numWheels'].name; // 'CastError' + // "pie" is not a number + err.errors['numWheels'].message; + // acquit:ignore:start + assert.equal(err.errors['numWheels'].name, 'CastError'); + assert.equal(err.errors['numWheels'].message, + '"pie" is not a number'); + db.deleteModel(/Vehicle/); + // acquit:ignore:end + }); + + /* eslint-disable no-unused-vars */ + it('Cast Error Message Function Overwrite', function() { + const vehicleSchema = new mongoose.Schema({ + numWheels: { + type: Number, + cast: [null, (value, path, model, kind) => `"${value}" is not a number`] + } + }); + const Vehicle = db.model('Vehicle', vehicleSchema); + + const doc = new Vehicle({ numWheels: 'pie' }); + const err = doc.validateSync(); + + err.errors['numWheels'].name; // 'CastError' + // "pie" is not a number + err.errors['numWheels'].message; + // acquit:ignore:start + assert.equal(err.errors['numWheels'].name, 'CastError'); + assert.equal(err.errors['numWheels'].message, + '"pie" is not a number'); + db.deleteModel(/Vehicle/); + // acquit:ignore:end + }); + it('Global SchemaType Validation', async function() { // Add a custom validator to all strings mongoose.Schema.Types.String.set('validate', v => v == null || v > 0); diff --git a/types/schematypes.d.ts b/types/schematypes.d.ts index b02f71a0744..74f96a2afc0 100644 --- a/types/schematypes.d.ts +++ b/types/schematypes.d.ts @@ -64,10 +64,10 @@ declare module 'mongoose' { /** Allows overriding casting logic for this individual path. If a string, the given string overwrites Mongoose's default cast error message. */ cast?: string | - boolean | - ((value: any) => T) | - [(value: any) => T, string] | - [((value: any) => T) | null, (value: any, path: string, model: Model, kind: string) => string]; + boolean | + ((value: any) => T) | + [(value: any) => T, string] | + [((value: any) => T) | null, (value: any, path: string, model: Model, kind: string) => string]; /** * If true, attach a required validator to this path, which ensures this path From 96ff8ab42f7f9e8e41fef3952987ed2583ced63f Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 14 Jul 2023 16:06:07 -0400 Subject: [PATCH 32/34] fix lint, try fixing ts benchmark blowup --- docs/validation.md | 6 +++--- types/schematypes.d.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/validation.md b/docs/validation.md index 5e7c87ff4d4..43ae5ceaef6 100644 --- a/docs/validation.md +++ b/docs/validation.md @@ -128,9 +128,9 @@ You can overwrite Mongoose's default cast error message by the `cast` option on Mongoose's cast error message templating supports the following parameters: -- `{PATH}`: the path that failed to cast -- `{VALUE}`: a string representation of the value that failed to cast -- `{KIND}`: the type that Mongoose attempted to cast to, like `'String'` or `'Number'` +* `{PATH}`: the path that failed to cast +* `{VALUE}`: a string representation of the value that failed to cast +* `{KIND}`: the type that Mongoose attempted to cast to, like `'String'` or `'Number'` You can also define a function that Mongoose will call to get the cast error message as follows. diff --git a/types/schematypes.d.ts b/types/schematypes.d.ts index 74f96a2afc0..02b2fddf7a8 100644 --- a/types/schematypes.d.ts +++ b/types/schematypes.d.ts @@ -67,7 +67,7 @@ declare module 'mongoose' { boolean | ((value: any) => T) | [(value: any) => T, string] | - [((value: any) => T) | null, (value: any, path: string, model: Model, kind: string) => string]; + [((value: any) => T) | null, (value: any, path: string, model: Model, kind: string) => string]; /** * If true, attach a required validator to this path, which ensures this path From 5db1d0d9f117e6bfedb87d0183e913abbdb7b049 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 14 Jul 2023 16:10:34 -0400 Subject: [PATCH 33/34] test: fix #3162 tests --- test/docs/validation.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/docs/validation.test.js b/test/docs/validation.test.js index a03cf494571..20e654a4f34 100644 --- a/test/docs/validation.test.js +++ b/test/docs/validation.test.js @@ -14,6 +14,8 @@ describe('validation docs', function() { }); }); + beforeEach(() => db.deleteModel(/Vehicle/)); + after(async function() { await db.close(); }); From 8378c82ed619bd2ce29899b52184609db632d6ce Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 18 Jul 2023 16:33:05 -0400 Subject: [PATCH 34/34] types: allow any value for $meta because MongoDB now supports values other than "textScore" for $meta --- types/query.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/query.d.ts b/types/query.d.ts index ab05d38dd83..e60846e60a6 100644 --- a/types/query.d.ts +++ b/types/query.d.ts @@ -691,7 +691,7 @@ declare module 'mongoose' { slice(val: number | Array): this; /** Sets the sort order. If an object is passed, values allowed are `asc`, `desc`, `ascending`, `descending`, `1`, and `-1`. */ - sort(arg?: string | { [key: string]: SortOrder | { $meta: 'textScore' } } | [string, SortOrder][] | undefined | null): this; + sort(arg?: string | { [key: string]: SortOrder | { $meta: any } } | [string, SortOrder][] | undefined | null): this; /** Sets the tailable option (for use with capped collections). */ tailable(bool?: boolean, opts?: {