From 8655c6c8ac72e3acb083a6539f50f6c538b22fe6 Mon Sep 17 00:00:00 2001 From: dragontaek-lee Date: Tue, 24 Sep 2024 22:28:35 +0900 Subject: [PATCH 1/4] fix(model): skip applying static hooks by default if static name conflicts with model,document middleware --- lib/constants.js | 27 +++++++++++++++++++++++++++ lib/helpers/model/applyStaticHooks.js | 18 +++++++++++------- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/lib/constants.js b/lib/constants.js index f5f5c5a19b..3a03bd502f 100644 --- a/lib/constants.js +++ b/lib/constants.js @@ -44,3 +44,30 @@ const aggregateMiddlewareFunctions = [ ]; exports.aggregateMiddlewareFunctions = aggregateMiddlewareFunctions; + +/*! + * ignore + */ + +const modelMiddlewareFunctions = [ + 'bulkWrite', + 'createCollection', + 'insertMany' +]; + +exports.modelMiddlewareFunctions = modelMiddlewareFunctions; + +/*! + * ignore + */ + +const documentMiddlewareFunctions = [ + 'validate', + 'save', + 'remove', + 'updateOne', + 'deleteOne', + 'init' +]; + +exports.documentMiddlewareFunctions = documentMiddlewareFunctions; diff --git a/lib/helpers/model/applyStaticHooks.js b/lib/helpers/model/applyStaticHooks.js index 34611c3b39..8e2c33a3bf 100644 --- a/lib/helpers/model/applyStaticHooks.js +++ b/lib/helpers/model/applyStaticHooks.js @@ -1,11 +1,15 @@ 'use strict'; const promiseOrCallback = require('../promiseOrCallback'); -const { queryMiddlewareFunctions, aggregateMiddlewareFunctions } = require('../../constants'); +const { queryMiddlewareFunctions, aggregateMiddlewareFunctions, modelMiddlewareFunctions, documentMiddlewareFunctions } = require('../../constants'); const middlewareFunctions = [ - ...queryMiddlewareFunctions, - ...aggregateMiddlewareFunctions + ...[ + ...queryMiddlewareFunctions, + ...aggregateMiddlewareFunctions, + ...modelMiddlewareFunctions, + ...documentMiddlewareFunctions + ].reduce((s, hook) => s.add(hook), new Set()) ]; module.exports = function applyStaticHooks(model, hooks, statics) { @@ -14,8 +18,11 @@ module.exports = function applyStaticHooks(model, hooks, statics) { numCallbackParams: 1 }; + model.$__insertMany = hooks.createWrapper('insertMany', + model.$__insertMany, model, kareemOptions); + hooks = hooks.filter(hook => { - // If the custom static overwrites an existing query/aggregate middleware, don't apply + // If the custom static overwrites an existing middleware, don't apply // middleware to it by default. This avoids a potential backwards breaking // change with plugins like `mongoose-delete` that use statics to overwrite // built-in Mongoose functions. @@ -25,9 +32,6 @@ module.exports = function applyStaticHooks(model, hooks, statics) { return hook.model !== false; }); - model.$__insertMany = hooks.createWrapper('insertMany', - model.$__insertMany, model, kareemOptions); - for (const key of Object.keys(statics)) { if (hooks.hasHooks(key)) { const original = model[key]; From c647a051c5dcf61d0a3123dc28502afd39331fd1 Mon Sep 17 00:00:00 2001 From: dragontaek-lee Date: Tue, 24 Sep 2024 22:29:46 +0900 Subject: [PATCH 2/4] fix(model): skip applying static hooks by default if static name conflicts with model,document middleware --- test/model.test.js | 48 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/test/model.test.js b/test/model.test.js index c2e3f68c1e..f162ea8f00 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -5891,6 +5891,54 @@ describe('Model', function() { assert.equal(called, 1); }); + it('custom statics that overwrite model functions dont get hooks by default', async function() { + + const schema = new Schema({ name: String }); + + schema.statics.insertMany = function(docs) { + return model.insertMany.apply(this, [docs]); + }; + + let called = 0; + schema.pre('insertMany', function(next) { + ++called; + next(); + }); + const Model = db.model('Test', schema); + + const res = await Model.insertMany([ + { name: 'foo' }, + { name: 'boo' } + ]); + + assert.ok(res[0].name); + assert.ok(res[1].name); + assert.equal(called, 1); + }); + + it('custom statics that overwrite document functions dont get hooks by default', async function() { + + const schema = new Schema({ name: String }); + + schema.statics.save = async function() { + return 'foo'; + }; + + let called = 0; + schema.pre('save', function(next) { + ++called; + next(); + }); + + const Model = db.model('Test', schema); + + const doc = await Model.save(); + + assert.ok(doc); + assert.equal(doc, 'foo'); + assert.equal(called, 0); + }); + it('error handling middleware passes saved doc (gh-7832)', async function() { const schema = new Schema({ _id: Number }); From a06debe2363490bb140001a570ced20a4a0227e8 Mon Sep 17 00:00:00 2001 From: dragontaek-lee Date: Tue, 24 Sep 2024 23:46:34 +0900 Subject: [PATCH 3/4] fix: remove redundant async --- test/model.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/model.test.js b/test/model.test.js index f162ea8f00..cca70e32fd 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -5920,7 +5920,7 @@ describe('Model', function() { const schema = new Schema({ name: String }); - schema.statics.save = async function() { + schema.statics.save = function() { return 'foo'; }; From 1f7c742c31e9763ca4ffa65bbd799d46aba81d57 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 25 Sep 2024 10:36:33 -0400 Subject: [PATCH 4/4] Update applyStaticHooks.js --- lib/helpers/model/applyStaticHooks.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/helpers/model/applyStaticHooks.js b/lib/helpers/model/applyStaticHooks.js index 8e2c33a3bf..3d0e129716 100644 --- a/lib/helpers/model/applyStaticHooks.js +++ b/lib/helpers/model/applyStaticHooks.js @@ -3,14 +3,14 @@ const promiseOrCallback = require('../promiseOrCallback'); const { queryMiddlewareFunctions, aggregateMiddlewareFunctions, modelMiddlewareFunctions, documentMiddlewareFunctions } = require('../../constants'); -const middlewareFunctions = [ - ...[ +const middlewareFunctions = Array.from( + new Set([ ...queryMiddlewareFunctions, ...aggregateMiddlewareFunctions, ...modelMiddlewareFunctions, ...documentMiddlewareFunctions - ].reduce((s, hook) => s.add(hook), new Set()) -]; + ]) +); module.exports = function applyStaticHooks(model, hooks, statics) { const kareemOptions = {