From c409b2c25584438a38e2498b2b100b89769b3a04 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sat, 24 Dec 2022 13:06:42 -0500 Subject: [PATCH 1/3] fix(discriminator): apply built-in plugins to discriminator schema even if `mergeHooks` and `mergePlugins` are both false Fix #12696 --- lib/helpers/model/discriminator.js | 3 +++ lib/helpers/schema/applyBuiltinPlugins.js | 12 ++++++++++ lib/plugins/clearValidating.js | 28 ----------------------- lib/plugins/index.js | 7 ++++++ test/model.discriminator.test.js | 24 +++++++++++++++++++ 5 files changed, 46 insertions(+), 28 deletions(-) create mode 100644 lib/helpers/schema/applyBuiltinPlugins.js delete mode 100644 lib/plugins/clearValidating.js create mode 100644 lib/plugins/index.js diff --git a/lib/helpers/model/discriminator.js b/lib/helpers/model/discriminator.js index 96443002ab1..0c843df10da 100644 --- a/lib/helpers/model/discriminator.js +++ b/lib/helpers/model/discriminator.js @@ -1,6 +1,7 @@ 'use strict'; const Mixed = require('../../schema/mixed'); +const applyBuiltinPlugins = require('../schema/applyBuiltinPlugins'); const defineKey = require('../document/compile').defineKey; const get = require('../get'); const utils = require('../../utils'); @@ -40,6 +41,8 @@ module.exports = function discriminator(model, name, schema, tiedValue, applyPlu model.base._applyPlugins(schema, { skipTopLevel: !applyPluginsToDiscriminators }); + } else if (!mergeHooks) { + applyBuiltinPlugins(schema); } const key = model.schema.options.discriminatorKey; diff --git a/lib/helpers/schema/applyBuiltinPlugins.js b/lib/helpers/schema/applyBuiltinPlugins.js new file mode 100644 index 00000000000..cbf999dd01d --- /dev/null +++ b/lib/helpers/schema/applyBuiltinPlugins.js @@ -0,0 +1,12 @@ +'use strict'; + +const builtinPlugins = require('../../plugins'); + +module.exports = function applyBuiltinPlugins(schema) { + for (const plugin of Object.values(builtinPlugins)) { + plugin(schema, { deduplicate: true }); + } + schema.plugins = Object.values(builtinPlugins). + map(fn => ({ fn, opts: { deduplicate: true } })). + concat(schema.plugins); +}; \ No newline at end of file diff --git a/lib/plugins/clearValidating.js b/lib/plugins/clearValidating.js deleted file mode 100644 index 50264e33a6b..00000000000 --- a/lib/plugins/clearValidating.js +++ /dev/null @@ -1,28 +0,0 @@ -'use strict'; - -/*! - * ignore - */ - -module.exports = function clearValidating(schema) { - // `this.$__.validating` tracks whether there are multiple validations running - // in parallel. We need to clear `this.$__.validating` before post hooks for gh-8597 - const unshift = true; - schema.s.hooks.post('validate', false, function clearValidatingPostValidate() { - if (this.$isSubdocument) { - return; - } - - this.$__.validating = null; - }, unshift); - - schema.s.hooks.post('validate', false, function clearValidatingPostValidateError(error, res, next) { - if (this.$isSubdocument) { - next(); - return; - } - - this.$__.validating = null; - next(); - }, unshift); -}; diff --git a/lib/plugins/index.js b/lib/plugins/index.js new file mode 100644 index 00000000000..69fa6ad284c --- /dev/null +++ b/lib/plugins/index.js @@ -0,0 +1,7 @@ +'use strict'; + +exports.removeSubdocs = require('./removeSubdocs'); +exports.saveSubdocs = require('./saveSubdocs'); +exports.sharding = require('./sharding'); +exports.trackTransaction = require('./trackTransaction'); +exports.validateBeforeSave = require('./validateBeforeSave'); diff --git a/test/model.discriminator.test.js b/test/model.discriminator.test.js index 3e34bfa7b62..a7eb5469a32 100644 --- a/test/model.discriminator.test.js +++ b/test/model.discriminator.test.js @@ -2077,4 +2077,28 @@ describe('model', function() { schema.pre('save', function testHook12604() {}); } }); + + it('applies built-in plugins if mergePlugins and mergeHooks disabled (gh-12696) (gh-12604)', async function() { + const shapeDef = { name: String }; + const shapeSchema = Schema(shapeDef, { discriminatorKey: 'kind' }); + + const Shape = db.model('Test', shapeSchema); + + let subdocSaveCalls = 0; + const nestedSchema = Schema({ test: String }); + nestedSchema.pre('save', function() { + ++subdocSaveCalls; + }); + + const squareSchema = Schema({ ...shapeDef, nested: nestedSchema }); + const Square = Shape.discriminator( + 'Square', + squareSchema, + { mergeHooks: false, mergePlugins: false } + ); + + assert.equal(subdocSaveCalls, 0); + await Square.create({ nested: { test: 'foo' } }); + assert.equal(subdocSaveCalls, 1); + }); }); From e64f652af89a086963797278d5957490b02f917d Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 26 Dec 2022 17:05:40 -0500 Subject: [PATCH 2/3] Update lib/helpers/schema/applyBuiltinPlugins.js Co-authored-by: hasezoey --- lib/helpers/schema/applyBuiltinPlugins.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/helpers/schema/applyBuiltinPlugins.js b/lib/helpers/schema/applyBuiltinPlugins.js index cbf999dd01d..8bd7319cbb1 100644 --- a/lib/helpers/schema/applyBuiltinPlugins.js +++ b/lib/helpers/schema/applyBuiltinPlugins.js @@ -9,4 +9,4 @@ module.exports = function applyBuiltinPlugins(schema) { schema.plugins = Object.values(builtinPlugins). map(fn => ({ fn, opts: { deduplicate: true } })). concat(schema.plugins); -}; \ No newline at end of file +}; From ffefa87c895287826e3f39178b3f6e9492121e9e Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 26 Dec 2022 17:09:28 -0500 Subject: [PATCH 3/3] refactor: use builtinPlugins list instead of importing all plugins --- lib/helpers/schema/applyBuiltinPlugins.js | 2 +- lib/index.js | 14 ++------------ 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/lib/helpers/schema/applyBuiltinPlugins.js b/lib/helpers/schema/applyBuiltinPlugins.js index cbf999dd01d..8bd7319cbb1 100644 --- a/lib/helpers/schema/applyBuiltinPlugins.js +++ b/lib/helpers/schema/applyBuiltinPlugins.js @@ -9,4 +9,4 @@ module.exports = function applyBuiltinPlugins(schema) { schema.plugins = Object.values(builtinPlugins). map(fn => ({ fn, opts: { deduplicate: true } })). concat(schema.plugins); -}; \ No newline at end of file +}; diff --git a/lib/index.js b/lib/index.js index 0b76506cb0e..2109d3eec6d 100644 --- a/lib/index.js +++ b/lib/index.js @@ -19,21 +19,17 @@ const Types = require('./types'); const Query = require('./query'); const Model = require('./model'); const applyPlugins = require('./helpers/schema/applyPlugins'); +const builtinPlugins = require('./plugins'); const driver = require('./driver'); const promiseOrCallback = require('./helpers/promiseOrCallback'); const legacyPluralize = require('./helpers/pluralize'); const utils = require('./utils'); const pkg = require('../package.json'); const cast = require('./cast'); -const removeSubdocs = require('./plugins/removeSubdocs'); -const saveSubdocs = require('./plugins/saveSubdocs'); -const trackTransaction = require('./plugins/trackTransaction'); -const validateBeforeSave = require('./plugins/validateBeforeSave'); const Aggregate = require('./aggregate'); const PromiseProvider = require('./promise_provider'); const printStrictQueryWarning = require('./helpers/printStrictQueryWarning'); -const shardingPlugin = require('./plugins/sharding'); const trusted = require('./helpers/query/trusted').trusted; const sanitizeFilter = require('./helpers/query/sanitizeFilter'); const isBsonType = require('./helpers/isBsonType'); @@ -108,13 +104,7 @@ function Mongoose(options) { configurable: false, enumerable: true, writable: false, - value: [ - [saveSubdocs, { deduplicate: true }], - [validateBeforeSave, { deduplicate: true }], - [shardingPlugin, { deduplicate: true }], - [removeSubdocs, { deduplicate: true }], - [trackTransaction, { deduplicate: true }] - ] + value: Object.values(builtinPlugins).map(plugin => ([plugin, { deduplicate: true }])) }); }