From 997f9f598651d31f273e1215d7e303445b0c254b Mon Sep 17 00:00:00 2001 From: Hafez Date: Fri, 21 Jan 2022 11:14:32 +0200 Subject: [PATCH 01/13] refactor tests to async/await --- test/model.indexes.test.js | 39 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/test/model.indexes.test.js b/test/model.indexes.test.js index a8b85daf128..91195b76b94 100644 --- a/test/model.indexes.test.js +++ b/test/model.indexes.test.js @@ -436,32 +436,30 @@ describe('model', function() { }); }); - it('skips automatic indexing on childSchema if autoIndex: false (gh-9150)', function() { + it('skips automatic indexing on childSchema if autoIndex: false (gh-9150)', async function() { const nestedSchema = mongoose.Schema({ name: { type: String, index: true } }, { autoIndex: false }); + const schema = mongoose.Schema({ nested: nestedSchema, top: { type: String, index: true } }); - let Model; - - return Promise.resolve(). - then(() => { - Model = db.model('Model', schema); - return Model.init(); - }). - then(() => Model.listIndexes()). - then(indexes => { - assert.equal(indexes.length, 2); - assert.deepEqual(indexes[1].key, { top: 1 }); - }); + + const Model = db.model('Model', schema); + + await Model.init(); + + const indexes = await Model.listIndexes(); + + assert.equal(indexes.length, 2); + assert.deepEqual(indexes[1].key, { top: 1 }); }); describe('discriminators with unique', function() { this.timeout(5000); - it('converts to partial unique index (gh-6347)', function() { + it('converts to partial unique index (gh-6347)', async function() { const baseOptions = { discriminatorKey: 'kind' }; const baseSchema = new Schema({}, baseOptions); @@ -483,19 +481,18 @@ describe('model', function() { const Device = Base.discriminator('Device', deviceSchema); - return Promise.all([ + await Promise.all([ Base.init(), User.init(), Device.init(), Base.create({}), User.create({ emailId: 'val@karpov.io', firstName: 'Val' }), Device.create({ name: 'Samsung', model: 'Galaxy' }) - ]).then(() => Base.listIndexes()). - then(indexes => indexes.find(i => i.key.other)). - then(index => { - assert.deepEqual(index.key, { other: 1 }); - assert.deepEqual(index.partialFilterExpression, { kind: 'Device' }); - }); + ]); + const indexes = await Base.listIndexes(); + const index = indexes.find(i => i.key.other); + assert.deepEqual(index.key, { other: 1 }); + assert.deepEqual(index.partialFilterExpression, { kind: 'Device' }); }); it('decorated discriminator index with syncIndexes (gh-6347)', function() { From 8a9608913cb55589c79e494c141401d8e4dba024 Mon Sep 17 00:00:00 2001 From: Hafez Date: Fri, 21 Jan 2022 11:18:03 +0200 Subject: [PATCH 02/13] fix test that always passes --- test/index.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/index.test.js b/test/index.test.js index b791ff8900a..1189c89737a 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -798,10 +798,10 @@ describe('mongoose module:', function() { await m.disconnect(); }); - it('connect with url doesnt cause unhandled rejection (gh-6997)', function() { + it('connect with url doesnt cause unhandled rejection (gh-6997)', async function() { const m = new mongoose.Mongoose; const _options = Object.assign({}, options, { serverSelectionTimeoutMS: 100 }); - const error = m.connect('mongodb://doesnotexist:27009/test', _options).then(() => null, err => err); + const error = await m.connect('mongodb://doesnotexist:27009/test', _options).then(() => null, err => err); assert.ok(error); }); From 616f3e058da745a5cda9cea6bdbe06462ad89352 Mon Sep 17 00:00:00 2001 From: Hafez Date: Fri, 21 Jan 2022 11:18:14 +0200 Subject: [PATCH 03/13] refactor test to async/await --- test/index.test.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/test/index.test.js b/test/index.test.js index 1189c89737a..60a14f3abf1 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -178,7 +178,7 @@ describe('mongoose module:', function() { assert.strictEqual(o._id, o); }); - it('runValidators option (gh-6865) (gh-6578)', function() { + it('runValidators option (gh-6865) (gh-6578)', async function() { const mongoose = new Mongoose(); mongoose.set('runValidators', true); @@ -187,13 +187,12 @@ describe('mongoose module:', function() { name: { type: String, required: true } })); - return mongoose.connect(uri, options). - then(() => M.updateOne({}, { name: null })). - then( - () => assert.ok(false), - err => assert.ok(err.errors['name']) - ). - then(() => mongoose.disconnect()); + await mongoose.connect(uri, options); + + const err = await M.updateOne({}, { name: null }).then(() => null, err => err); + assert.ok(err.errors['name']); + + mongoose.disconnect(); }); it('toJSON options (gh-6815)', function() { From 52c84ea7a47f0620d402f3c88186407d5358fbbe Mon Sep 17 00:00:00 2001 From: Hafez Date: Mon, 24 Jan 2022 20:28:25 +0200 Subject: [PATCH 04/13] test(connection): repro #11039 --- test/connection.test.js | 236 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 216 insertions(+), 20 deletions(-) diff --git a/test/connection.test.js b/test/connection.test.js index f091d5701c0..539b682f404 100644 --- a/test/connection.test.js +++ b/test/connection.test.js @@ -1079,27 +1079,223 @@ describe('connections:', function() { assert.deepStrictEqual(conn3.id, m.connection.id + 2); }); - it('Allows a syncIndexes option with connection mongoose.connection.syncIndexes (gh-10893)', async function() { - this.timeout(10000); + describe('Connection#syncIndexes() (gh-10893) (gh-11039)', () => { + let connection; + this.beforeEach(async() => { + const mongooseInstance = new mongoose.Mongoose(); + connection = mongooseInstance.createConnection('mongodb://localhost:27017/connection_sync_indexes_test'); + }); + this.afterEach(async() => { + await connection.dropDatabase(); + }); + it('Allows a syncIndexes option with connection mongoose.connection.syncIndexes (gh-10893)', async function() { + const coll = 'tests2'; + + let User = connection.model('Test', new Schema({ name: { type: String, index: true } }, { autoIndex: false }), coll); + const indexesAfterFirstSync = await connection.syncIndexes(); + assert.deepEqual(indexesAfterFirstSync, { Test: [] }); + + const indexesAfterSecondSync = await User.listIndexes(); + assert.deepEqual( + indexesAfterSecondSync.map(i => i.key), + [{ _id: 1 }, { name: 1 }] + ); + + connection.deleteModel(/Test/); + + User = connection.model('Test', new Schema({ + otherName: { type: String, index: true } + }, { autoIndex: false }), coll); + + const indexesAfterDropping = await connection.syncIndexes(); + assert.deepEqual(indexesAfterDropping, { Test: ['name_1'] }); + }); + it('does not sync indexes automatically when `autoIndex: true` (gh-11039)', async function() { + // Arrange + const buildingSchema = new Schema({ name: String }, { autoIndex: false }); + buildingSchema.index({ name: 1 }); + const Building = connection.model('Building', buildingSchema); + + const floorSchema = new Schema({ name: String }, { autoIndex: false }); + floorSchema.index({ name: 1 }, { unique: true }); + const Floor = connection.model('Floor', floorSchema); + + const officeSchema = new Schema({ name: String }, { autoIndex: false }); + const Office = connection.model('Office', officeSchema); + + // ensure all colections exist + // otherwise `listIndexes` randomly fails because a collection hasn't been created in the database yt. + await Promise.all([ + Building.createCollection(), + Floor.createCollection(), + Office.createCollection() + ]); + + // Act + const [ + buildingIndexes, + floorIndexes, + officeIndexes + ] = await Promise.all([ + Building.listIndexes(), + Floor.listIndexes(), + Office.listIndexes() + ]); + + // Assert + assert.deepEqual(buildingIndexes.map(index => index.key), [{ _id: 1 }]); + assert.deepEqual(floorIndexes.map(index => index.key), [{ _id: 1 }]); + assert.deepEqual(officeIndexes.map(index => index.key), [{ _id: 1 }]); + }); + it('stops as soon as one model fails with `continueOnError: false` (gh-11039)', async function() { + // Arrange + const buildingSchema = new Schema({ name: String }, { autoIndex: false }); + buildingSchema.index({ name: 1 }); + const Building = connection.model('Building', buildingSchema); + + const floorSchema = new Schema({ name: String }, { autoIndex: false }); + floorSchema.index({ name: 1 }, { unique: true }); + const Floor = connection.model('Floor', floorSchema); + + const officeSchema = new Schema({ name: String }, { autoIndex: false }); + officeSchema.index({ name: 1 }); + const Office = connection.model('Office', officeSchema); + await Floor.insertMany([ + { name: 'I am a duplicate' }, + { name: 'I am a duplicate' } + ]); + + // Act + const err = await connection.syncIndexes().then(() => null, err => err); + assert.ok(err); + + + // Assert + const [ + buildingIndexes, + floorIndexes, + officeIndexes + ] = await Promise.all([ + Building.listIndexes(), + Floor.listIndexes(), + Office.listIndexes() + ]); + + assert.deepEqual(buildingIndexes.map(index => index.key), [{ _id: 1 }, { name: 1 }]); + assert.deepEqual(floorIndexes.map(index => index.key), [{ _id: 1 }]); + assert.deepEqual(officeIndexes.map(index => index.key), [{ _id: 1 }]); + }); + + it('error includes a property with all the errors when `continueOnError: false`', async() => { + // Arrange + const bookSchema = new Schema({ name: String }, { autoIndex: false }); + bookSchema.index({ name: 1 }, { unique: true }); + const Book = connection.model('Book', bookSchema); + + + await Book.insertMany([ + { name: 'I am a duplicate' }, + { name: 'I am a duplicate' } + ]); + + // Act + const err = await connection.syncIndexes({ continueOnError: false }).then(() => null, err => err); + // Assert + assert.ok(err); + assert.ok(err.message.includes('E11000')); + assert.equal(err.name, 'SyncIndexesError'); + assert.equal(err.errors['Book'].code, 11000); + assert.equal(err.errors['Book'].code, 11000); + }); + + it('`continueOnError` is false by default', async() => { + // Arrange + const bookSchema = new Schema({ name: String }, { autoIndex: false }); + bookSchema.index({ name: 1 }, { unique: true }); + const Book = connection.model('Book', bookSchema); + + + await Book.insertMany([ + { name: 'I am a duplicate' }, + { name: 'I am a duplicate' } + ]); + + // Act + const err = await connection.syncIndexes().then(() => null, err => err); + + // Assert + assert.ok(err); + assert.ok(err.message.includes('E11000')); + assert.equal(err.name, 'SyncIndexesError'); + assert.equal(err.errors['Book'].code, 11000); + assert.equal(err.errors['Book'].code, 11000); + }); + it('when `continueOnError: true` it will continue to sync indexes even if one model fails', async() => { + // Arrange + const buildingSchema = new Schema({ name: String }, { autoIndex: false }); + buildingSchema.index({ name: 1 }); + const Building = connection.model('Building', buildingSchema); + + const floorSchema = new Schema({ name: String }, { autoIndex: false }); + floorSchema.index({ name: 1 }, { unique: true }); + const Floor = connection.model('Floor', floorSchema); + + const officeSchema = new Schema({ name: String }, { autoIndex: false }); + officeSchema.index({ name: 1 }); + const Office = connection.model('Office', officeSchema); + + await Floor.insertMany([ + { name: 'I am a duplicate' }, + { name: 'I am a duplicate' } + ]); + + // Act + await connection.syncIndexes({ continueOnError: true }); + + // Assert + const [ + buildingIndexes, + floorIndexes, + officeIndexes + ] = await Promise.all([ + Building.listIndexes(), + Floor.listIndexes(), + Office.listIndexes() + ]); + + assert.deepEqual(buildingIndexes.map(index => index.key), [{ _id: 1 }, { name: 1 }]); + assert.deepEqual(floorIndexes.map(index => index.key), [{ _id: 1 }]); + assert.deepEqual(officeIndexes.map(index => index.key), [{ _id: 1 }, { name: 1 }]); + }); - const coll = 'tests2'; - const mongooseInstance = new mongoose.Mongoose(); - const conn = mongooseInstance.createConnection('mongodb://localhost:27017'); - let User = conn.model('Test', new Schema({ name: { type: String, index: true } }, { autoIndex: false }), coll); - const indexesAfterFirstSync = await conn.syncIndexes(); - assert(indexesAfterFirstSync, { Test: [] }); - const indexesAfterSecondSync = await User.listIndexes(); - assert.deepEqual(indexesAfterSecondSync.map(i => i.key), [ - { _id: 1 }, - { name: 1 } - ]); - conn.deleteModel(/Test/); - User = conn.model('Test', new Schema({ - otherName: { type: String, index: true } - }, { autoIndex: false }), coll); - const dropped = await conn.syncIndexes(); - assert.deepEqual(dropped, { Test: ['name_1'] }); - await User.collection.drop(); + it('when `continueOnError: true` it will return a map of modelNames and their sync results/errors', async() => { + // Arrange + const buildingSchema = new Schema({ name: String }, { autoIndex: false }); + buildingSchema.index({ name: 1 }); + connection.model('Building', buildingSchema); + + const floorSchema = new Schema({ name: String }, { autoIndex: false }); + floorSchema.index({ name: 1 }, { unique: true }); + const Floor = connection.model('Floor', floorSchema); + + const officeSchema = new Schema({ name: String }, { autoIndex: false }); + officeSchema.index({ name: 1 }); + connection.model('Office', officeSchema); + + await Floor.insertMany([ + { name: 'I am a duplicate' }, + { name: 'I am a duplicate' } + ]); + + // Act + const result = await connection.syncIndexes({ continueOnError: true }); + + // Assert + assert.ok(Array.isArray(result['Building'])); + assert.ok(result['Floor'].name, 'MongoServerError'); + assert.ok(Array.isArray(result['Office'])); + }); }); + }); From c1fa15f52745d1381915b08908bbb5a9287988f4 Mon Sep 17 00:00:00 2001 From: Hafez Date: Mon, 24 Jan 2022 20:53:13 +0200 Subject: [PATCH 05/13] enhancement(connection): add support for `continueOnError` --- index.d.ts | 14 ++++++++++---- lib/connection.js | 25 ++++++++++++++++++++++--- lib/error/syncIndexes.js | 30 ++++++++++++++++++++++++++++++ test/typescript/connection.ts | 4 ++++ 4 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 lib/error/syncIndexes.js diff --git a/index.d.ts b/index.d.ts index ffc8bb26da8..f88d214c33d 100644 --- a/index.d.ts +++ b/index.d.ts @@ -68,14 +68,20 @@ declare module 'mongoose' { export function connect(uri: string, callback: CallbackWithoutResult): void; export function connect(uri: string, options?: ConnectOptions): Promise; + export interface SyncIndexesOptions { + continueOnError?: boolean + } + export type SyncIndexesResult = Array| Record | SyncIndexesError> + + /** * Makes the indexes in MongoDB match the indexes defined in every model's * schema. This function will drop any indexes that are not defined in * the model's schema except the `_id` index, and build any indexes that * are in your schema but not in MongoDB. */ - export function syncIndexes(options?: Record): Promise>; - export function syncIndexes(options: Record | null, callback: Callback>): void; + export function syncIndexes(options?:SyncIndexesOptions): Promise; + export function syncIndexes(options: SyncIndexesOptions | null, callback: Callback): void; /* Tells `sanitizeFilter()` to skip the given object when filtering out potential query selector injection attacks. * Use this method when you have a known query selector that you want to use. */ @@ -930,8 +936,8 @@ declare module 'mongoose' { * the model's schema except the `_id` index, and build any indexes that * are in your schema but not in MongoDB. */ - syncIndexes(options?: Record): Promise>; - syncIndexes(options: Record | null, callback: Callback>): void; + syncIndexes(options?: SyncIndexesOptions): Promise | >; + syncIndexes(options: SyncIndexesOptions | null, callback: Callback>): void; /** * Does a dry-run of Model.syncIndexes(), meaning that diff --git a/lib/connection.js b/lib/connection.js index cd16a60ab11..7ff5b144938 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -10,6 +10,7 @@ const Schema = require('./schema'); const Collection = require('./driver').get().Collection; const STATES = require('./connectionstate'); const MongooseError = require('./error/index'); +const SyncIndexesError = require('./error/syncIndexes'); const PromiseProvider = require('./promise_provider'); const ServerSelectionError = require('./error/serverSelection'); const applyPlugins = require('./helpers/schema/applyPlugins'); @@ -1376,11 +1377,29 @@ Connection.prototype.setClient = function setClient(client) { return this; }; -Connection.prototype.syncIndexes = async function syncIndexes() { +Connection.prototype.syncIndexes = async function syncIndexes({ continueOnError } = { continueOnError: false }) { const result = {}; - for (const model in this.models) { - result[model] = await this.model(model).syncIndexes(); + const errorsMap = { }; + + for (const model of Object.values(this.models)) { + try { + result[model.modelName] = await model.syncIndexes(); + } catch (err) { + if (!continueOnError) { + errorsMap[model.modelName] = err; + break; + } else { + result[model.modelName] = err; + } + } + } + + if (!continueOnError && Object.keys(errorsMap).length) { + const message = Object.values(errorsMap).map(err => err.message).join(' '); + const syncIndexesError = new SyncIndexesError(message, errorsMap); + throw syncIndexesError; } + return result; }; diff --git a/lib/error/syncIndexes.js b/lib/error/syncIndexes.js new file mode 100644 index 00000000000..35cffcb9425 --- /dev/null +++ b/lib/error/syncIndexes.js @@ -0,0 +1,30 @@ +'use strict'; + +/*! + * Module dependencies. + */ + +const MongooseError = require('./mongooseError'); + +/** + * SyncIndexes Error constructor. + * + * @param {String} type + * @param {String} value + * @inherits MongooseError + * @api private + */ + +class SyncIndexesError extends MongooseError { + constructor(message, errorsMap) { + super(message); + this.errors = errorsMap; + } +} + +Object.defineProperty(SyncIndexesError.prototype, 'name', { + value: 'SyncIndexesError' +}); + + +module.exports = SyncIndexesError; diff --git a/test/typescript/connection.ts b/test/typescript/connection.ts index 1618b7d192e..503db0c4f8f 100644 --- a/test/typescript/connection.ts +++ b/test/typescript/connection.ts @@ -14,3 +14,7 @@ createConnection('mongodb://localhost:27017/test').close(); conn.db.collection('Test').findOne({ name: String }).then(doc => console.log(doc)); conn.collection('Test').findOne({ name: String }).then(doc => console.log(doc)); +conn.syncIndexes({ continueOnError: false }).then(result => { + const a = result['Test'][0]; + a['Test'].name; +}); \ No newline at end of file From fb74bd448717e8646c9f38fc31cc8d05d57489ec Mon Sep 17 00:00:00 2001 From: Hafez Date: Tue, 25 Jan 2022 12:42:58 +0200 Subject: [PATCH 06/13] revert TS changes for syncIndexes --- index.d.ts | 21 ++++++++------------- test/typescript/connection.ts | 5 ++--- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/index.d.ts b/index.d.ts index f88d214c33d..25e3e48617a 100644 --- a/index.d.ts +++ b/index.d.ts @@ -68,20 +68,14 @@ declare module 'mongoose' { export function connect(uri: string, callback: CallbackWithoutResult): void; export function connect(uri: string, options?: ConnectOptions): Promise; - export interface SyncIndexesOptions { - continueOnError?: boolean - } - export type SyncIndexesResult = Array| Record | SyncIndexesError> - - /** * Makes the indexes in MongoDB match the indexes defined in every model's * schema. This function will drop any indexes that are not defined in * the model's schema except the `_id` index, and build any indexes that * are in your schema but not in MongoDB. */ - export function syncIndexes(options?:SyncIndexesOptions): Promise; - export function syncIndexes(options: SyncIndexesOptions | null, callback: Callback): void; + export function syncIndexes(options?: Record): Promise>; + export function syncIndexes(options: Record | null, callback: Callback>): void; /* Tells `sanitizeFilter()` to skip the given object when filtering out potential query selector injection attacks. * Use this method when you have a known query selector that you want to use. */ @@ -455,7 +449,7 @@ declare module 'mongoose' { * _Requires MongoDB >= 3.6.0._ Executes the wrapped async function * in a transaction. Mongoose will commit the transaction if the * async function executes successfully and attempt to retry if - * there was a retriable error. + * there was a retryable error. */ transaction(fn: (session: mongodb.ClientSession) => Promise): Promise; @@ -936,8 +930,8 @@ declare module 'mongoose' { * the model's schema except the `_id` index, and build any indexes that * are in your schema but not in MongoDB. */ - syncIndexes(options?: SyncIndexesOptions): Promise | >; - syncIndexes(options: SyncIndexesOptions | null, callback: Callback>): void; + syncIndexes(options?: Record): Promise>; + syncIndexes(options: Record | null, callback: Callback>): void; /** * Does a dry-run of Model.syncIndexes(), meaning that @@ -2130,6 +2124,7 @@ declare module 'mongoose' { type QueryWithHelpers = Query & THelpers; type UnpackedIntersection = T extends (infer V)[] ? (V & U)[] : T & U; + type UnpackedIntersectionWithNull = T extends null ? UnpackedIntersection | null : UnpackedIntersection; class Query { _mongooseOptions: MongooseQueryOptions; @@ -2413,8 +2408,8 @@ declare module 'mongoose' { polygon(path: string, ...coordinatePairs: number[][]): this; /** Specifies paths which should be populated with other documents. */ - populate(path: string | any, select?: string | any, model?: string | Model, match?: any): QueryWithHelpers, DocType, THelpers, RawDocType>; - populate(options: PopulateOptions | Array): QueryWithHelpers, DocType, THelpers, RawDocType>; + populate(path: string | any, select?: string | any, model?: string | Model, match?: any): QueryWithHelpers, DocType, THelpers, RawDocType>; + populate(options: PopulateOptions | Array): QueryWithHelpers, DocType, THelpers, RawDocType>; /** Get/set the current projection (AKA fields). Pass `null` to remove the current projection. */ projection(fields?: any | null): this; diff --git a/test/typescript/connection.ts b/test/typescript/connection.ts index 503db0c4f8f..645e2709797 100644 --- a/test/typescript/connection.ts +++ b/test/typescript/connection.ts @@ -14,7 +14,6 @@ createConnection('mongodb://localhost:27017/test').close(); conn.db.collection('Test').findOne({ name: String }).then(doc => console.log(doc)); conn.collection('Test').findOne({ name: String }).then(doc => console.log(doc)); -conn.syncIndexes({ continueOnError: false }).then(result => { - const a = result['Test'][0]; - a['Test'].name; +conn.syncIndexes().then(result => { + result; }); \ No newline at end of file From a769b0bebe0da50a0dc7d226ae4650ac73851e3e Mon Sep 17 00:00:00 2001 From: Hafez Date: Tue, 25 Jan 2022 13:37:02 +0200 Subject: [PATCH 07/13] test(base): support `continueOnError` for mongoose.syncIndexes --- test/connection.test.js | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/connection.test.js b/test/connection.test.js index 539b682f404..e09f590ef51 100644 --- a/test/connection.test.js +++ b/test/connection.test.js @@ -1296,6 +1296,37 @@ describe('connections:', function() { assert.ok(result['Floor'].name, 'MongoServerError'); assert.ok(Array.isArray(result['Office'])); }); + + it('mongoose.syncIndexes(...) accepts `continueOnError`', async() => { + const m = new mongoose.Mongoose; + await m.connect('mongodb://localhost:27017/connection_sync_indexes_test'); + + // Arrange + const buildingSchema = new Schema({ name: String }, { autoIndex: false }); + buildingSchema.index({ name: 1 }); + m.model('Building', buildingSchema); + + const floorSchema = new Schema({ name: String }, { autoIndex: false }); + floorSchema.index({ name: 1 }, { unique: true }); + const Floor = m.model('Floor', floorSchema); + + const officeSchema = new Schema({ name: String }, { autoIndex: false }); + officeSchema.index({ name: 1 }); + m.model('Office', officeSchema); + + await Floor.insertMany([ + { name: 'I am a duplicate' }, + { name: 'I am a duplicate' } + ]); + + // Act + const result = await m.syncIndexes({ continueOnError: true }); + + // Assert + assert.ok(Array.isArray(result['Building'])); + assert.ok(result['Floor'].name, 'MongoServerError'); + assert.ok(Array.isArray(result['Office'])); + }); }); }); From 6275960abfaae6d1e71e43655b71b4fb2c4ddb39 Mon Sep 17 00:00:00 2001 From: Hafez Date: Tue, 25 Jan 2022 13:37:26 +0200 Subject: [PATCH 08/13] enhancement(base & connection): support `continueOnError` for syncIndexes --- lib/connection.js | 8 ++++++++ lib/index.js | 12 ++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/connection.js b/lib/connection.js index 7ff5b144938..07bbf45de9d 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -1377,6 +1377,14 @@ Connection.prototype.setClient = function setClient(client) { return this; }; +/** + * + * Syncs all the indexes for the models registered with this connection. + * + * @param {Object} options + * @param {Boolean} options.continueOnError `false` by default. If set to `true`, mongoose will not throw an error if one model syncing failed, and will return an object where the keys are the names of the models, and the values are the results/errors for each model. + * @returns + */ Connection.prototype.syncIndexes = async function syncIndexes({ continueOnError } = { continueOnError: false }) { const result = {}; const errorsMap = { }; diff --git a/lib/index.js b/lib/index.js index 5b6bfbaaeb6..7a3c3a6ed6f 100644 --- a/lib/index.js +++ b/lib/index.js @@ -993,9 +993,17 @@ Mongoose.prototype.isValidObjectId = function(v) { return false; }; -Mongoose.prototype.syncIndexes = function() { +/** + * + * Syncs all the indexes for the models registered with this connection. + * + * @param {Object} options + * @param {Boolean} options.continueOnError `false` by default. If set to `true`, mongoose will not throw an error if one model syncing failed, and will return an object where the keys are the names of the models, and the values are the results/errors for each model. + * @returns + */ +Mongoose.prototype.syncIndexes = function({ continueOnError } = { continueOnError: false }) { const _mongoose = this instanceof Mongoose ? this : mongoose; - return _mongoose.connection.syncIndexes(); + return _mongoose.connection.syncIndexes({ continueOnError }); }; /** From 2084075f04bd68b8430d0a465b1575285018377b Mon Sep 17 00:00:00 2001 From: Hafez Date: Tue, 25 Jan 2022 13:37:38 +0200 Subject: [PATCH 09/13] refactor test --- test/model.test.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test/model.test.js b/test/model.test.js index 4994b0e580c..544ade2a2c0 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -6134,13 +6134,13 @@ describe('Model', function() { const coll = 'tests' + random(); - let M = db.model('Test', new Schema({ + let Model = db.model('Test', new Schema({ name: { type: String, index: true } }, { autoIndex: false }), coll); - let dropped = await M.syncIndexes(); + let dropped = await Model.syncIndexes(); assert.deepEqual(dropped, []); - let indexes = await M.listIndexes(); + let indexes = await Model.listIndexes(); assert.deepEqual(indexes.map(i => i.key), [ { _id: 1 }, { name: 1 } @@ -6148,14 +6148,14 @@ describe('Model', function() { // New model, same collection, index on different property db.deleteModel(/Test/); - M = db.model('Test', new Schema({ + Model = db.model('Test', new Schema({ otherName: { type: String, index: true } }, { autoIndex: false }), coll); - dropped = await M.syncIndexes(); + dropped = await Model.syncIndexes(); assert.deepEqual(dropped, ['name_1']); - indexes = await M.listIndexes(); + indexes = await Model.listIndexes(); assert.deepEqual(indexes.map(i => i.key), [ { _id: 1 }, { otherName: 1 } @@ -6163,24 +6163,24 @@ describe('Model', function() { // New model, same collection, different options db.deleteModel(/Test/); - M = db.model('Test', new Schema({ + Model = db.model('Test', new Schema({ otherName: { type: String, unique: true } }, { autoIndex: false }), coll); - dropped = await M.syncIndexes(); + dropped = await Model.syncIndexes(); assert.deepEqual(dropped, ['otherName_1']); - indexes = await M.listIndexes(); + indexes = await Model.listIndexes(); assert.deepEqual(indexes.map(i => i.key), [ { _id: 1 }, { otherName: 1 } ]); // Re-run syncIndexes(), shouldn't change anything - dropped = await M.syncIndexes(); + dropped = await Model.syncIndexes(); assert.deepEqual(dropped, []); - await M.collection.drop(); + await Model.collection.drop(); }); From b5470f2404992fbd6fdea45daadd15cd8b0e8e59 Mon Sep 17 00:00:00 2001 From: Hafez Date: Fri, 28 Jan 2022 01:16:58 +0200 Subject: [PATCH 10/13] fix(index.d.ts): add types for base and connection syncIndexes(...) --- index.d.ts | 20 ++++++++++++++++---- lib/error/syncIndexes.js | 4 ++-- test/typescript/connection.ts | 11 +++++++++-- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/index.d.ts b/index.d.ts index 25e3e48617a..51e928e338b 100644 --- a/index.d.ts +++ b/index.d.ts @@ -74,8 +74,8 @@ declare module 'mongoose' { * the model's schema except the `_id` index, and build any indexes that * are in your schema but not in MongoDB. */ - export function syncIndexes(options?: Record): Promise>; - export function syncIndexes(options: Record | null, callback: Callback>): void; + export function syncIndexes(options?: SyncIndexesOptions): Promise; + export function syncIndexes(options: SyncIndexesOptions | null, callback: Callback): void; /* Tells `sanitizeFilter()` to skip the given object when filtering out potential query selector injection attacks. * Use this method when you have a known query selector that you want to use. */ @@ -165,6 +165,7 @@ declare module 'mongoose' { export const version: string; export type CastError = Error.CastError; + export type SyncIndexesError = Error.SyncIndexesError; type Mongoose = typeof mongoose; @@ -442,8 +443,8 @@ declare module 'mongoose' { * the model's schema except the `_id` index, and build any indexes that * are in your schema but not in MongoDB. */ - syncIndexes(options?: Record): Promise>; - syncIndexes(options: Record | null, callback: Callback>): void; + syncIndexes(options?: SyncIndexesOptions): Promise; + syncIndexes(options: SyncIndexesOptions | null, callback: Callback): void; /** * _Requires MongoDB >= 3.6.0._ Executes the wrapped async function @@ -3303,6 +3304,11 @@ declare module 'mongoose' { type?: string): this; } + export interface SyncIndexesOptions { + continueOnError?: boolean + } + export type ConnectionSyncIndexesResult = Record; + type OneCollectionSyncIndexesResult = Array & mongodb.MongoServerError; type Callback = (error: CallbackError, result: T) => void; type CallbackWithoutResult = (error: CallbackError) => void; @@ -3333,6 +3339,12 @@ declare module 'mongoose' { constructor(type: string, value: any, path: string, reason?: NativeError, schemaType?: SchemaType); } + export class SyncIndexesError extends Error { + name: 'SyncIndexesError'; + errors?: Record; + + constructor(type: string, value: any, path: string, reason?: NativeError, schemaType?: SchemaType); + } export class DisconnectedError extends Error { name: 'DisconnectedError'; diff --git a/lib/error/syncIndexes.js b/lib/error/syncIndexes.js index 35cffcb9425..54f345ba701 100644 --- a/lib/error/syncIndexes.js +++ b/lib/error/syncIndexes.js @@ -9,8 +9,8 @@ const MongooseError = require('./mongooseError'); /** * SyncIndexes Error constructor. * - * @param {String} type - * @param {String} value + * @param {String} message + * @param {String} errorsMap * @inherits MongooseError * @api private */ diff --git a/test/typescript/connection.ts b/test/typescript/connection.ts index 645e2709797..e388e4362c7 100644 --- a/test/typescript/connection.ts +++ b/test/typescript/connection.ts @@ -14,6 +14,13 @@ createConnection('mongodb://localhost:27017/test').close(); conn.db.collection('Test').findOne({ name: String }).then(doc => console.log(doc)); conn.collection('Test').findOne({ name: String }).then(doc => console.log(doc)); -conn.syncIndexes().then(result => { - result; +conn.syncIndexes({ continueOnError: true }).then(result => { + result['User'].forEach((index) => { + index.includes('name'); + }); + result['Order'].message; + result['Order'].code; +}); +conn.syncIndexes({ continueOnError: false }).then().catch(err => { + err.errors['Order'].code; }); \ No newline at end of file From b383e4682b13be92d601092525070c61fa6908f7 Mon Sep 17 00:00:00 2001 From: Hafez Date: Sat, 29 Jan 2022 23:29:22 +0200 Subject: [PATCH 11/13] fix review comments --- index.d.ts | 2 +- lib/connection.js | 11 +++++++---- lib/index.js | 4 ++-- test/typescript/connection.ts | 2 +- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/index.d.ts b/index.d.ts index 51e928e338b..2fa23eccdf7 100644 --- a/index.d.ts +++ b/index.d.ts @@ -3304,7 +3304,7 @@ declare module 'mongoose' { type?: string): this; } - export interface SyncIndexesOptions { + export interface SyncIndexesOptions extends mongodb.CreateIndexesOptions { continueOnError?: boolean } export type ConnectionSyncIndexesResult = Record; diff --git a/lib/connection.js b/lib/connection.js index 6846b48c870..0d18bfe7bb0 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -1385,17 +1385,20 @@ Connection.prototype.setClient = function setClient(client) { * * Syncs all the indexes for the models registered with this connection. * - * @param {Object} options + * @param {import('mongodb').CreateIndexesOptions & { continueOnError?: boolean } } options * @param {Boolean} options.continueOnError `false` by default. If set to `true`, mongoose will not throw an error if one model syncing failed, and will return an object where the keys are the names of the models, and the values are the results/errors for each model. * @returns */ -Connection.prototype.syncIndexes = async function syncIndexes({ continueOnError } = { continueOnError: false }) { +Connection.prototype.syncIndexes = async function syncIndexes(options = {}) { const result = {}; const errorsMap = { }; + const { continueOnError } = options; + delete options.continueOnError; + for (const model of Object.values(this.models)) { try { - result[model.modelName] = await model.syncIndexes(); + result[model.modelName] = await model.syncIndexes(options); } catch (err) { if (!continueOnError) { errorsMap[model.modelName] = err; @@ -1407,7 +1410,7 @@ Connection.prototype.syncIndexes = async function syncIndexes({ continueOnError } if (!continueOnError && Object.keys(errorsMap).length) { - const message = Object.values(errorsMap).map(err => err.message).join(' '); + const message = Object.entries(([modelName, err]) => `${modelName}: ${err.message}`).join(', '); const syncIndexesError = new SyncIndexesError(message, errorsMap); throw syncIndexesError; } diff --git a/lib/index.js b/lib/index.js index 7a3c3a6ed6f..39f7b05d44b 100644 --- a/lib/index.js +++ b/lib/index.js @@ -1001,9 +1001,9 @@ Mongoose.prototype.isValidObjectId = function(v) { * @param {Boolean} options.continueOnError `false` by default. If set to `true`, mongoose will not throw an error if one model syncing failed, and will return an object where the keys are the names of the models, and the values are the results/errors for each model. * @returns */ -Mongoose.prototype.syncIndexes = function({ continueOnError } = { continueOnError: false }) { +Mongoose.prototype.syncIndexes = function(options) { const _mongoose = this instanceof Mongoose ? this : mongoose; - return _mongoose.connection.syncIndexes({ continueOnError }); + return _mongoose.connection.syncIndexes(options); }; /** diff --git a/test/typescript/connection.ts b/test/typescript/connection.ts index e388e4362c7..a8fabba80c6 100644 --- a/test/typescript/connection.ts +++ b/test/typescript/connection.ts @@ -21,6 +21,6 @@ conn.syncIndexes({ continueOnError: true }).then(result => { result['Order'].message; result['Order'].code; }); -conn.syncIndexes({ continueOnError: false }).then().catch(err => { +conn.syncIndexes({ continueOnError: false, background: true }).then().catch(err => { err.errors['Order'].code; }); \ No newline at end of file From cc6ac731561f10449dc768bca6fb2c71a31d61ef Mon Sep 17 00:00:00 2001 From: Hafez Date: Sat, 29 Jan 2022 23:34:31 +0200 Subject: [PATCH 12/13] revert JSDoc usage of TS types due to weird docs result --- lib/connection.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/connection.js b/lib/connection.js index 0d18bfe7bb0..4d9034e692c 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -1385,7 +1385,7 @@ Connection.prototype.setClient = function setClient(client) { * * Syncs all the indexes for the models registered with this connection. * - * @param {import('mongodb').CreateIndexesOptions & { continueOnError?: boolean } } options + * @param {Object} options * @param {Boolean} options.continueOnError `false` by default. If set to `true`, mongoose will not throw an error if one model syncing failed, and will return an object where the keys are the names of the models, and the values are the results/errors for each model. * @returns */ From a367262327d6f829dfe26beffab32a0efb624e33 Mon Sep 17 00:00:00 2001 From: Hafez Date: Mon, 31 Jan 2022 02:36:23 +0200 Subject: [PATCH 13/13] fix failing tests --- lib/connection.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/connection.js b/lib/connection.js index 4d9034e692c..24f260ab751 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -1410,7 +1410,7 @@ Connection.prototype.syncIndexes = async function syncIndexes(options = {}) { } if (!continueOnError && Object.keys(errorsMap).length) { - const message = Object.entries(([modelName, err]) => `${modelName}: ${err.message}`).join(', '); + const message = Object.entries(errorsMap).map(([modelName, err]) => `${modelName}: ${err.message}`).join(', '); const syncIndexesError = new SyncIndexesError(message, errorsMap); throw syncIndexesError; }