diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d6b09dc2a11..4cb2d684a6f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -41,14 +41,10 @@ jobs: matrix: node: [12, 14, 16, 18] os: [ubuntu-18.04, ubuntu-20.04] - mongodb: [4.0.2, 5.0.2, 6.0.0] + mongodb: [4.0.28, 5.0.8, 6.0.0] include: - - os: ubuntu-18.04 - mongodb-os: ubuntu1804 - - os: ubuntu-20.04 - mongodb-os: ubuntu2004 - os: ubuntu-20.04 # customize on which matrix the coverage will be collected on - mongodb: 5.0.2 + mongodb: 5.0.11 node: 16 coverage: true exclude: @@ -59,6 +55,9 @@ jobs: - os: ubuntu-20.04 # exclude because there are no <4.4 mongodb builds for 2004 mongodb: 4.0.2 name: Node ${{ matrix.node }} MongoDB ${{ matrix.mongodb }} OS ${{ matrix.os }} + env: + MONGOMS_VERSION: ${{ matrix.mongodb }} + MONGOMS_PREFER_GLOBAL_PATH: 1 steps: - uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b # v3.0.2 @@ -67,18 +66,14 @@ jobs: with: node-version: ${{ matrix.node }} - - run: npm install + - name: Load MongoDB binary cache + id: cache-mongodb-binaries + uses: actions/cache@v3 + with: + path: ~/.cache/mongodb-binaries + key: ${{ matrix.os }}-${{ matrix.mongodb }} - - name: Setup - run: | - wget -q https://downloads.mongodb.org/linux/mongodb-linux-x86_64-${{ matrix.mongodb-os }}-${{ matrix.mongodb }}.tgz - tar xf mongodb-linux-x86_64-${{ matrix.mongodb-os }}-${{ matrix.mongodb }}.tgz - mkdir -p ./data/db/27017 ./data/db/27000 - printf "\ntimeout: 8000" >> ./.mocharc.yml - ./mongodb-linux-x86_64-${{ matrix.mongodb-os }}-${{ matrix.mongodb }}/bin/mongod --setParameter ttlMonitorSleepSecs=1 --fork --dbpath ./data/db/27017 --syslog --port 27017 - sleep 2 - mongod --version - echo `pwd`/mongodb-linux-x86_64-${{ matrix.mongodb-os }}-${{ matrix.mongodb }}/bin >> $GITHUB_PATH + - run: npm install - name: NPM Test without Coverage run: npm test if: matrix.coverage != true diff --git a/.mocharc.yml b/.mocharc.yml index 1810d04bb59..efa9bf8a87b 100644 --- a/.mocharc.yml +++ b/.mocharc.yml @@ -1,2 +1,4 @@ reporter: spec # better to identify failing / slow tests than "dot" ui: bdd # explicitly setting, even though it is mocha default +require: + - test/mocha-fixtures.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 03f4e0f0aa1..21f611b650f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +6.6.4 / 2022-10-03 +================== + * fix(model): avoid saving applied defaults if path is deselected #12506 #12414 + * fix(types): correct DocType for auto typed query helpers #12342 + * fix(types): avoid "excessively deep" type instantiation error when using bulkWrite() with type that extends from document #12277 + * fix(types): avoid relying on typeof this, which isn't supported in TypeScript < 4.4 #12375 + * docs(schema): correct example for Schema.prototype.discriminator() #12493 + * docs(typescript): clean up query helpers examples #12342 + * chore: use mongodb-memory-server for testing #12262 [hasezoey](https://github.com/hasezoey) + 6.6.3 / 2022-09-30 ================== * fix(query): treat findOne(_id) as equivalent to findOne({ _id }) #12485 #12325 diff --git a/docs/typescript/query-helpers.md b/docs/typescript/query-helpers.md index 7c108d465f2..de6bb1daf60 100644 --- a/docs/typescript/query-helpers.md +++ b/docs/typescript/query-helpers.md @@ -3,6 +3,8 @@ [Query helpers](http://thecodebarbarian.com/mongoose-custom-query-methods.html) let you define custom helper methods on Mongoose queries. Query helpers make queries more semantic using chaining syntax. +The following is an example of how query helpers work in JavaScript. + ```javascript ProjectSchema.query.byName = function(name) { return this.find({ name: name }); @@ -14,9 +16,9 @@ var Project = mongoose.model('Project', ProjectSchema); Project.find().where('stars').gt(1000).byName('mongoose'); ``` -In TypeScript, Mongoose does support manually typed and automatically typed Query Helpers. +## Manually Typed Query Helpers -1- Manually typed: +In TypeScript, you can define query helpers using a separate query helpers interface. Mongoose's `Model` takes 3 generic parameters: 1. The `DocType` @@ -30,23 +32,34 @@ Below is an example of creating a `ProjectModel` with a `byName` query helper. import { HydratedDocument, Model, Query, Schema, model } from 'mongoose'; interface Project { - name: string; - stars: number; + name?: string; + stars?: number; } -type ProjectModelType = Model; -// Query helpers should return `Query> & ProjectQueryHelpers` -// to enable chaining. -type ProjectModelQuery = Query, ProjectQueryHelpers> & ProjectQueryHelpers; interface ProjectQueryHelpers { - byName(this: ProjectModelQuery, name: string): ProjectModelQuery; + byName(name: string): QueryWithHelpers< + HydratedDocument[], + HydratedDocument, + ProjectQueryHelpers + > } -const schema = new Schema({ - name: { type: String, required: true }, - stars: { type: Number, required: true } +type ProjectModelType = Model; + +const ProjectSchema = new Schema< + Project, + Model, + {}, + ProjectQueryHelpers +>({ + name: String, + stars: Number }); -schema.query.byName = function(name: string): ProjectModelQuery { + +ProjectSchema.query.byName = function byName( + this: QueryWithHelpers, ProjectQueryHelpers>, + name: string +) { return this.find({ name: name }); }; @@ -63,30 +76,27 @@ async function run(): Promise { } ``` -2- Automatically typed: +## Auto Typed Query Helpers + Mongoose does support auto typed Query Helpers that it are supplied in schema options. Query Helpers functions can be defined as following: ```typescript import { Schema, model } from 'mongoose'; - const schema = new Schema({ - name: { type: String, required: true }, - stars: { type: Number, required: true } - }, { - query: { - byName(name) { - return this.find({ name: name }); - } +const ProjectSchema = new Schema({ + name: String, + stars: Number +}, { + query: { + byName(name: string) { + return this.find({ name }); } - }); + } +}); - const ProjectModel = model( - 'Project', - schema - ); +const ProjectModel = model('Project', ProjectSchema); - // Equivalent to `ProjectModel.find({ stars: { $gt: 1000 }, name: 'mongoose' })` - await ProjectModel.find().where('stars').gt(1000).byName('mongoose'); -} +// Equivalent to `ProjectModel.find({ stars: { $gt: 1000 }, name: 'mongoose' })` +await ProjectModel.find().where('stars').gt(1000).byName('mongoose'); ``` \ No newline at end of file diff --git a/lib/helpers/projection/isPathExcluded.js b/lib/helpers/projection/isPathExcluded.js index e0424b52365..e8f126b22da 100644 --- a/lib/helpers/projection/isPathExcluded.js +++ b/lib/helpers/projection/isPathExcluded.js @@ -12,6 +12,10 @@ const isDefiningProjection = require('./isDefiningProjection'); */ module.exports = function isPathExcluded(projection, path) { + if (projection == null) { + return false; + } + if (path === '_id') { return projection._id === 0; } diff --git a/lib/model.js b/lib/model.js index 9abbeeb14f0..9c3bb658b04 100644 --- a/lib/model.js +++ b/lib/model.js @@ -51,11 +51,13 @@ const { getRelatedDBIndexes, getRelatedSchemaIndexes } = require('./helpers/indexes/getRelatedIndexes'); +const isPathExcluded = require('./helpers/projection/isPathExcluded'); const decorateDiscriminatorIndexOptions = require('./helpers/indexes/decorateDiscriminatorIndexOptions'); const isPathSelectedInclusive = require('./helpers/projection/isPathSelectedInclusive'); const leanPopulateMap = require('./helpers/populate/leanPopulateMap'); const modifiedPaths = require('./helpers/update/modifiedPaths'); const parallelLimit = require('./helpers/parallelLimit'); +const parentPaths = require('./helpers/path/parentPaths'); const prepareDiscriminatorPipeline = require('./helpers/aggregate/prepareDiscriminatorPipeline'); const pushNestedArrayPaths = require('./helpers/model/pushNestedArrayPaths'); const removeDeselectedForeignField = require('./helpers/populate/removeDeselectedForeignField'); @@ -760,6 +762,19 @@ Model.prototype.$__delta = function() { } } + // If this path is set to default, and either this path or one of + // its parents is excluded, don't treat this path as dirty. + if (this.$isDefault(data.path) && this.$__.selected) { + if (data.path.indexOf('.') === -1 && isPathExcluded(this.$__.selected, data.path)) { + continue; + } + + const pathsToCheck = parentPaths(data.path); + if (pathsToCheck.find(path => isPathExcluded(this.$__.isSelected, path))) { + continue; + } + } + if (divergent.length) continue; if (value === undefined) { operand(this, where, delta, data, 1, '$unset'); @@ -798,6 +813,11 @@ Model.prototype.$__delta = function() { if (this.$__.version) { this.$__version(where, delta); } + + if (Object.keys(delta).length === 0) { + return [where, null]; + } + return [where, delta]; }; @@ -1196,7 +1216,6 @@ Model.exists = function exists(filter, options, callback) { */ Model.discriminator = function(name, schema, options) { - let model; if (typeof name === 'function') { model = name; diff --git a/lib/schema.js b/lib/schema.js index 1c5195fffe8..62061f97ed9 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -507,25 +507,21 @@ Schema.prototype.defaultOptions = function(options) { * * #### Example: * - * const options = { discriminatorKey: 'kind' }; + * const eventSchema = new mongoose.Schema({ timestamp: Date }, { discriminatorKey: 'kind' }); + * + * const clickedEventSchema = new mongoose.Schema({ element: String }, { discriminatorKey: 'kind' }); + * const ClickedModel = eventSchema.discriminator('clicked', clickedEventSchema); * - * const eventSchema = new mongoose.Schema({ time: Date }, options); * const Event = mongoose.model('Event', eventSchema); * - * // ClickedLinkEvent is a special type of Event that has - * // a URL. - * const ClickedLinkEvent = Event.discriminator('ClickedLink', - * new mongoose.Schema({ url: String }, options)); + * Event.discriminators['clicked']; // Model { clicked } * - * // When you create a generic event, it can't have a URL field... - * const genericEvent = new Event({ time: Date.now(), url: 'google.com' }); - * assert.ok(!genericEvent.url); - * // But a ClickedLinkEvent can - * const clickedEvent = new ClickedLinkEvent({ time: Date.now(), url: 'google.com' }); - * assert.ok(clickedEvent.url); + * const doc = await Event.create({ kind: 'clicked', element: '#hero' }); + * doc.element; // '#hero' + * doc instanceof ClickedModel; // true * * @param {String} name the name of the discriminator - * @param {Schema} schema the Schema of the discriminated Schema + * @param {Schema} schema the discriminated Schema * @return {Schema} the Schema instance * @api public */ diff --git a/package.json b/package.json index 841840ff780..ccd4e5786fa 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "mongoose", "description": "Mongoose MongoDB ODM", - "version": "6.6.3", + "version": "6.6.4", "author": "Guillermo Rauch ", "keywords": [ "mongodb", diff --git a/test/common.js b/test/common.js index 932ca2d1b37..6e036983c91 100644 --- a/test/common.js +++ b/test/common.js @@ -114,11 +114,15 @@ module.exports = function(options) { return conn; }; -function getUri(env, default_uri, db) { +function getUri(default_uri, db) { + const env = process.env.START_REPLICA_SET ? process.env.MONGOOSE_REPLSET_URI : process.env.MONGOOSE_TEST_URI; const use = env ? env : default_uri; const lastIndex = use.lastIndexOf('/'); + const dbQueryString = use.slice(lastIndex); + const queryIndex = dbQueryString.indexOf('?'); + const query = queryIndex === -1 ? '' : '?' + dbQueryString.slice(queryIndex + 1); // use length if lastIndex is 9 or lower, because that would mean it found the last character of "mongodb://" - return use.slice(0, lastIndex <= 9 ? use.length : lastIndex) + `/${db}`; + return use.slice(0, lastIndex <= 9 ? use.length : lastIndex) + `/${db}` + query; } /** @@ -134,13 +138,18 @@ const databases = module.exports.databases = [ * testing uri */ -module.exports.uri = getUri(process.env.MONGOOSE_TEST_URI, 'mongodb://127.0.0.1:27017/', databases[0]); +// the following has to be done, otherwise mocha will evaluate this before running the global-setup, where it becomes the default +Object.defineProperty(module.exports, 'uri', { + get: () => getUri('mongodb://127.0.0.1:27017/', databases[0]) +}); /** * testing uri for 2nd db */ -module.exports.uri2 = getUri(process.env.MONGOOSE_TEST_URI, 'mongodb://127.0.0.1:27017/', databases[1]); +Object.defineProperty(module.exports, 'uri2', { + get: () => getUri('mongodb://127.0.0.1:27017/', databases[1]) +}); /** * expose mongoose @@ -177,28 +186,13 @@ module.exports.mongodVersion = async function() { }; async function dropDBs() { + this.timeout(60000); + const db = await module.exports({ noErrorListener: true }).asPromise(); await db.dropDatabase(); await db.close(); } -before(async function() { - this.timeout(60000); - if (process.env.START_REPLICA_SET) { - const uri = await startReplicaSet(); - - module.exports.uri = uri; - module.exports.uri2 = uri.replace(databases[0], databases[1]); - - process.env.REPLICA_SET = 'rs0'; - - const conn = mongoose.createConnection(uri); - await conn.asPromise(); - await conn.db.collection('test').findOne(); - await conn.close(); - } -}); - before(dropDBs); after(dropDBs); @@ -212,34 +206,3 @@ process.on('unhandledRejection', function(error, promise) { } throw error; }); - -async function startReplicaSet() { - const ReplSet = require('mongodb-memory-server').MongoMemoryReplSet; - - // Create new instance - const replSet = new ReplSet({ - binary: { - version: '5.0.4' - }, - instanceOpts: [ - // Set the expiry job in MongoDB to run every second - { - port: 27017, - args: ['--setParameter', 'ttlMonitorSleepSecs=1'] - } - ], - dbName: databases[0], - replSet: { - name: 'rs0', - count: 2, - storageEngine: 'wiredTiger' - } - }); - - await replSet.start(); - await replSet.waitUntilRunning(); - - await new Promise(resolve => setTimeout(resolve, 10000)); - - return replSet.getUri(databases[0]); -} diff --git a/test/document.test.js b/test/document.test.js index 84cafc22463..ca2aca278d9 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -11881,6 +11881,24 @@ describe('document', function() { const addedDoc = await Model.findOne({ name: 'Test' }); assert.strictEqual(addedDoc.count, 1); }); + + it('avoids overwriting array if saving with no changes with array deselected (gh-12414)', async function() { + const schema = new mongoose.Schema({ + name: String, + tags: [String] + }); + const Test = db.model('Test', schema); + + const { _id } = await Test.create({ name: 'Mongoose', tags: ['mongodb'] }); + + const doc = await Test.findById(_id).select('name'); + assert.deepStrictEqual(doc.getChanges(), {}); + await doc.save(); + + const rawDoc = await Test.collection.findOne({ _id }); + assert.ok(rawDoc); + assert.deepStrictEqual(rawDoc.tags, ['mongodb']); + }); }); describe('Check if instance function that is supplied in schema option is availabe', function() { diff --git a/test/index.test.js b/test/index.test.js index 2b5163d611e..723bc817234 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -715,13 +715,12 @@ describe('mongoose module:', function() { it('with replica set', async function() { const mong = new Mongoose(); - const uri = process.env.MONGOOSE_SET_TEST_URI; - if (!uri) { - return; + if (!start.uri) { + return this.skip(); } - await mong.connect(uri, options); + await mong.connect(start.uri, options); await mong.connection.close(); }); diff --git a/test/mocha-fixtures.js b/test/mocha-fixtures.js new file mode 100644 index 00000000000..2e537071c52 --- /dev/null +++ b/test/mocha-fixtures.js @@ -0,0 +1,56 @@ +'use strict'; + +const mms = require('mongodb-memory-server'); + +/* + * Default MMS mongodb version is used, unless MONGOMS_VERSION is set (which is set with the matrix in test.yml for CI) + */ + +// a single-instance running for single-instance tests +let mongoinstance; + +// a replset-instance for running repl-set tests +let mongorreplset; + +// decide wheter to start a in-memory or not +const startMemoryInstance = !process.env.MONGOOSE_TEST_URI; +const startMemoryReplset = !process.env.MONGOOSE_REPLSET_URI; + +module.exports.mochaGlobalSetup = async function mochaGlobalSetup() { + let instanceuri; + let replseturi; + + process.env.RUNTIME_DOWNLOAD = '1'; // ensure MMS is able to download binaries in this context + + // set some options when running in a CI + if (process.env.CI) { + process.env.MONGOMS_PREFER_GLOBAL_PATH = '1'; // set MMS to use "~/.cache/mongodb-binaries" even when the path does not yet exist + } + + if (startMemoryInstance) { // Config to decided if an mongodb-memory-server instance should be used + // it's needed in global space, because we don't want to create a new instance every test-suite + mongoinstance = await mms.MongoMemoryServer.create({ instance: { args: ['--setParameter', 'ttlMonitorSleepSecs=1'], storageEngine: 'wiredTiger' } }); + instanceuri = mongoinstance.getUri(); + } else { + instanceuri = process.env.MONGOOSE_TEST_URI; + } + + if (startMemoryReplset) { + mongorreplset = await mms.MongoMemoryReplSet.create({ replSet: { count: 3, args: ['--setParameter', 'ttlMonitorSleepSecs=1'], storageEngine: 'wiredTiger' } }); // using 3 because even numbers can lead to vote problems + replseturi = mongorreplset.getUri(); + } else { + replseturi = ''; + } + + process.env.MONGOOSE_TEST_URI = instanceuri; + process.env.MONGOOSE_REPLSET_URI = replseturi; +}; + +module.exports.mochaGlobalTeardown = async function mochaGlobalTeardown() { + if (mongoinstance) { // Config to decided if an mongodb-memory-server instance should be used + await mongoinstance.stop(); + } + if (mongorreplset) { + await mongorreplset.stop(); + } +}; diff --git a/test/types/models.test.ts b/test/types/models.test.ts index 2eff9d93b8a..19c598c63a4 100644 --- a/test/types/models.test.ts +++ b/test/types/models.test.ts @@ -10,7 +10,6 @@ import { CallbackError, HydratedDocument, HydratedDocumentFromSchema, - LeanDocument, Query, UpdateWriteOpResult } from 'mongoose'; @@ -314,6 +313,33 @@ function bulkWriteAddToSet() { return M.bulkWrite(ops); } +async function gh12277() { + type DocumentType = Document & T; + + interface BaseModelClassDoc { + firstname: string; + } + + const baseModelClassSchema = new Schema({ + firstname: String + }); + + const BaseModel = model>('test', baseModelClassSchema); + + await BaseModel.bulkWrite([ + { + updateOne: { + update: { + firstname: 'test' + }, + filter: { + firstname: 'asdsd' + } + } + } + ]); +} + export function autoTypedModel() { const AutoTypedSchema = autoTypedSchema(); const AutoTypedModel = model('AutoTypeModel', AutoTypedSchema); diff --git a/test/types/queries.test.ts b/test/types/queries.test.ts index 0725c42855b..239e5eeb488 100644 --- a/test/types/queries.test.ts +++ b/test/types/queries.test.ts @@ -359,3 +359,66 @@ function gh12142() { } ); } + +async function gh12342_manual() { + interface Project { + name?: string, stars?: number + } + + interface ProjectQueryHelpers { + byName(name: string): QueryWithHelpers< + HydratedDocument[], + HydratedDocument, + ProjectQueryHelpers + > + } + + type ProjectModelType = Model; + + const ProjectSchema = new Schema< + Project, + Model, + {}, + ProjectQueryHelpers + >({ + name: String, + stars: Number + }); + + ProjectSchema.query.byName = function byName( + this: QueryWithHelpers, ProjectQueryHelpers>, + name: string + ) { + return this.find({ name: name }); + }; + + // 2nd param to `model()` is the Model class to return. + const ProjectModel = model('Project', schema); + + expectType[]>( + await ProjectModel.findOne().where('stars').gt(1000).byName('mongoose') + ); +} + +async function gh12342_auto() { + interface Project { + name?: string, stars?: number + } + + const ProjectSchema = new Schema({ + name: String, + stars: Number + }, { + query: { + byName(name: string) { + return this.find({ name }); + } + } + }); + + const ProjectModel = model('Project', ProjectSchema); + + expectType[]>( + await ProjectModel.findOne().where('stars').gt(1000).byName('mongoose') + ); +} diff --git a/types/models.d.ts b/types/models.d.ts index d2774b2c644..d3cbbcf1338 100644 --- a/types/models.d.ts +++ b/types/models.d.ts @@ -146,9 +146,9 @@ declare module 'mongoose' { * if you use `create()`) because with `bulkWrite()` there is only one network * round trip to the MongoDB server. */ - bulkWrite(writes: Array>, options: mongodb.BulkWriteOptions & MongooseBulkWriteOptions, callback: Callback): void; - bulkWrite(writes: Array>, callback: Callback): void; - bulkWrite(writes: Array>, options?: mongodb.BulkWriteOptions & MongooseBulkWriteOptions): Promise; + bulkWrite(writes: Array>, options: mongodb.BulkWriteOptions & MongooseBulkWriteOptions, callback: Callback): void; + bulkWrite(writes: Array>, callback: Callback): void; + bulkWrite(writes: Array>, options?: mongodb.BulkWriteOptions & MongooseBulkWriteOptions): Promise; /** * Sends multiple `save()` calls in a single `bulkWrite()`. This is faster than diff --git a/types/query.d.ts b/types/query.d.ts index 115fdc99cbe..3a3a7a6b36f 100644 --- a/types/query.d.ts +++ b/types/query.d.ts @@ -616,7 +616,7 @@ declare module 'mongoose' { then: Promise['then']; /** Converts this query to a customized, reusable query constructor with all arguments and options retained. */ - toConstructor(): typeof this; + toConstructor(): typeof Query; /** Declare and/or execute this query as an update() operation. */ update(filter?: FilterQuery, update?: UpdateQuery | UpdateWithAggregationPipeline, options?: QueryOptions | null, callback?: Callback): QueryWithHelpers; diff --git a/types/schemaoptions.d.ts b/types/schemaoptions.d.ts index 1a87a8ccf9f..c3d35bd4272 100644 --- a/types/schemaoptions.d.ts +++ b/types/schemaoptions.d.ts @@ -201,7 +201,7 @@ declare module 'mongoose' { /** * Query helper functions. */ - query?: Record>(this: T, ...args: any) => T> | QueryHelpers, + query?: Record>>(this: T, ...args: any) => T> | QueryHelpers, /** * Set whether to cast non-array values to arrays.