From 641250cea3f59c38f51b90355f35b96e21e4fbe8 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 1 Aug 2024 11:06:22 -0400 Subject: [PATCH 1/4] feat(model+query): support `options` parameter for distinct() Fix #8006 --- lib/model.js | 8 ++++++-- lib/query.js | 11 +++++++++-- test/docs/transactions.test.js | 20 ++++++++++++++++++++ test/query.test.js | 10 ++++++++++ test/types/queries.test.ts | 1 + types/models.d.ts | 3 ++- 6 files changed, 48 insertions(+), 5 deletions(-) diff --git a/lib/model.js b/lib/model.js index b3497669be5..7b9223d2408 100644 --- a/lib/model.js +++ b/lib/model.js @@ -2115,17 +2115,21 @@ Model.countDocuments = function countDocuments(conditions, options) { * * @param {String} field * @param {Object} [conditions] optional + * @param {Object} [options] optional * @return {Query} * @api public */ -Model.distinct = function distinct(field, conditions) { +Model.distinct = function distinct(field, conditions, options) { _checkContext(this, 'distinct'); - if (typeof arguments[0] === 'function' || typeof arguments[1] === 'function') { + if (typeof arguments[0] === 'function' || typeof arguments[1] === 'function' || typeof arguments[2] === 'function') { throw new MongooseError('Model.distinct() no longer accepts a callback'); } const mq = new this.Query({}, {}, this, this.$__collection); + if (options != null) { + mq.setOptions(options); + } return mq.distinct(field, conditions); }; diff --git a/lib/query.js b/lib/query.js index ccca65f4192..20532ad1f8f 100644 --- a/lib/query.js +++ b/lib/query.js @@ -2870,21 +2870,24 @@ Query.prototype.__distinct = async function __distinct() { * * #### Example: * + * distinct(field, conditions, options) * distinct(field, conditions) * distinct(field) * distinct() * * @param {String} [field] * @param {Object|Query} [filter] + * @param {Object} [options] * @return {Query} this * @see distinct https://www.mongodb.com/docs/manual/reference/method/db.collection.distinct/ * @api public */ -Query.prototype.distinct = function(field, conditions) { +Query.prototype.distinct = function(field, conditions, options) { if (typeof field === 'function' || typeof conditions === 'function' || - typeof arguments[2] === 'function') { + typeof options === 'function' || + typeof arguments[3] === 'function') { throw new MongooseError('Query.prototype.distinct() no longer accepts a callback'); } @@ -2903,6 +2906,10 @@ Query.prototype.distinct = function(field, conditions) { this._distinct = field; } + if (typeof options === 'object' && options != null) { + this.setOptions(options); + } + return this; }; diff --git a/test/docs/transactions.test.js b/test/docs/transactions.test.js index 2b19aef9ee3..b2e208fe17d 100644 --- a/test/docs/transactions.test.js +++ b/test/docs/transactions.test.js @@ -338,6 +338,26 @@ describe('transactions', function() { assert.deepEqual(fromDb, { name: 'Tyrion Lannister' }); }); + it('distinct (gh-8006)', async function() { + const Character = db.model('gh8006_Character', new Schema({ name: String, rank: String }, { versionKey: false })); + + + const session = await db.startSession(); + + session.startTransaction(); + await Character.create([{ name: 'Will Riker', rank: 'Commander' }, { name: 'Jean-Luc Picard', rank: 'Captain' }], { session }); + + let names = await Character.distinct('name', {}, { session }); + assert.deepStrictEqual(names.sort(), ['Jean-Luc Picard', 'Will Riker']); + + names = await Character.distinct('name', { rank: 'Captain' }, { session }); + assert.deepStrictEqual(names.sort(), ['Jean-Luc Picard']); + + // Undo both update and delete since doc should pull from `$session()` + await session.abortTransaction(); + session.endSession(); + }); + it('save() with no changes (gh-8571)', async function() { db.deleteModel(/Test/); const Test = db.model('Test', Schema({ name: String })); diff --git a/test/query.test.js b/test/query.test.js index 553ab04e152..4f75d0f2fab 100644 --- a/test/query.test.js +++ b/test/query.test.js @@ -1088,6 +1088,16 @@ describe('Query', function() { assert.equal(q.op, 'distinct'); }); + + it('using options parameter for distinct', function() { + const q = new Query({}); + const options = { collation: { locale: 'en', strength: 2 } }; + + q.distinct('blah', {}, options); + + assert.equal(q.op, 'distinct'); + assert.deepEqual(q.options.collation, options.collation); + }); }); describe('findOne', function() { diff --git a/test/types/queries.test.ts b/test/types/queries.test.ts index d84022526e4..5396f384cad 100644 --- a/test/types/queries.test.ts +++ b/test/types/queries.test.ts @@ -109,6 +109,7 @@ Test.find({ name: { $gte: 'Test' } }, null, { collation: { locale: 'en-us' } }). Test.findOne().orFail(new Error('bar')).then((doc: ITest | null) => console.log('Found! ' + doc)); Test.distinct('name').exec().then((res: Array) => console.log(res[0])); +Test.distinct('name', {}, { collation: { locale: 'en', strength: 2 } }).exec().then((res: Array) => console.log(res[0])); Test.findOneAndUpdate({ name: 'test' }, { name: 'test2' }).exec().then((res: ITest | null) => console.log(res)); Test.findOneAndUpdate({ name: 'test' }, { name: 'test2' }).then((res: ITest | null) => console.log(res)); diff --git a/types/models.d.ts b/types/models.d.ts index 27c43612ac2..c042305a828 100644 --- a/types/models.d.ts +++ b/types/models.d.ts @@ -621,7 +621,8 @@ declare module 'mongoose' { /** Creates a `distinct` query: returns the distinct values of the given `field` that match `filter`. */ distinct( field: DocKey, - filter?: FilterQuery + filter?: FilterQuery, + options?: QueryOptions ): QueryWithHelpers< Array< DocKey extends keyof WithLevel1NestedPaths From d94d8d36e2670d139c5e7694afc0359ab7108071 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 7 Aug 2024 19:56:55 -0400 Subject: [PATCH 2/4] Update test/docs/transactions.test.js Co-authored-by: hasezoey --- test/docs/transactions.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/docs/transactions.test.js b/test/docs/transactions.test.js index b2e208fe17d..de2ecfc9952 100644 --- a/test/docs/transactions.test.js +++ b/test/docs/transactions.test.js @@ -341,7 +341,6 @@ describe('transactions', function() { it('distinct (gh-8006)', async function() { const Character = db.model('gh8006_Character', new Schema({ name: String, rank: String }, { versionKey: false })); - const session = await db.startSession(); session.startTransaction(); From f6be45a95bfb8fb4c192e519059de83f459f1a25 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 7 Aug 2024 20:05:24 -0400 Subject: [PATCH 3/4] types(query): add options param to distinct() re: #8006 --- types/query.d.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/types/query.d.ts b/types/query.d.ts index 850d06eb25a..66a0042d342 100644 --- a/types/query.d.ts +++ b/types/query.d.ts @@ -354,7 +354,8 @@ declare module 'mongoose' { /** Creates a `distinct` query: returns the distinct values of the given `field` that match `filter`. */ distinct( field: DocKey, - filter?: FilterQuery + filter?: FilterQuery, + options?: QueryOptions ): QueryWithHelpers< Array< DocKey extends keyof WithLevel1NestedPaths From 58f384dbf04cc9df23e19113094b836ffa6438c7 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 7 Aug 2024 20:05:53 -0400 Subject: [PATCH 4/4] fix(query): consistent handling for non-object options with countDocuments(), estimatedDocumentCount(), distinct() --- lib/query.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/query.js b/lib/query.js index 20532ad1f8f..fa5d2cd856e 100644 --- a/lib/query.js +++ b/lib/query.js @@ -2773,7 +2773,7 @@ Query.prototype.estimatedDocumentCount = function(options) { this.op = 'estimatedDocumentCount'; this._validateOp(); - if (typeof options === 'object' && options != null) { + if (options != null) { this.setOptions(options); } @@ -2832,7 +2832,7 @@ Query.prototype.countDocuments = function(conditions, options) { this.merge(conditions); } - if (typeof options === 'object' && options != null) { + if (options != null) { this.setOptions(options); } @@ -2906,7 +2906,7 @@ Query.prototype.distinct = function(field, conditions, options) { this._distinct = field; } - if (typeof options === 'object' && options != null) { + if (options != null) { this.setOptions(options); }