From 57541cbefa33aaecd7b391e2f4d1b6d44805c7d3 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 13 Feb 2024 10:46:09 -0500 Subject: [PATCH 1/3] refactor: make query helpers more consistently fit our loose definition of helpers --- lib/aggregate.js | 4 +- lib/constants.js | 36 +++++++++++++ lib/helpers/model/applyStaticHooks.js | 2 +- lib/helpers/query/applyGlobalOption.js | 18 +++---- lib/helpers/query/applyQueryMiddleware.js | 54 -------------------- lib/helpers/query/castFilterPath.js | 3 +- lib/helpers/query/completeMany.js | 36 ------------- lib/helpers/query/validOps.js | 19 +------ lib/model.js | 38 +++++++++++++- lib/query.js | 61 +++++++++++++++++------ lib/schema.js | 3 +- 11 files changed, 133 insertions(+), 141 deletions(-) create mode 100644 lib/constants.js delete mode 100644 lib/helpers/query/applyQueryMiddleware.js delete mode 100644 lib/helpers/query/completeMany.js diff --git a/lib/aggregate.js b/lib/aggregate.js index e61daa8a5df..827f1642a60 100644 --- a/lib/aggregate.js +++ b/lib/aggregate.js @@ -1019,8 +1019,8 @@ Aggregate.prototype.exec = async function exec() { const model = this._model; const collection = this._model.collection; - applyGlobalMaxTimeMS(this.options, model); - applyGlobalDiskUse(this.options, model); + applyGlobalMaxTimeMS(this.options, model.db.options, model.base.options); + applyGlobalDiskUse(this.options, model.db.options, model.base.options); if (this.options && this.options.cursor) { return new AggregationCursor(this); diff --git a/lib/constants.js b/lib/constants.js new file mode 100644 index 00000000000..83a66832b55 --- /dev/null +++ b/lib/constants.js @@ -0,0 +1,36 @@ +'use strict'; + +/*! + * ignore + */ + +const queryOperations = Object.freeze([ + // Read + 'countDocuments', + 'distinct', + 'estimatedDocumentCount', + 'find', + 'findOne', + // Update + 'findOneAndReplace', + 'findOneAndUpdate', + 'replaceOne', + 'updateMany', + 'updateOne', + // Delete + 'deleteMany', + 'deleteOne', + 'findOneAndDelete' +]); + +exports.queryOperations = queryOperations; + +/*! + * ignore + */ + +const queryMiddlewareFunctions = queryOperations.concat([ + 'validate' +]); + +exports.queryMiddlewareFunctions = queryMiddlewareFunctions; diff --git a/lib/helpers/model/applyStaticHooks.js b/lib/helpers/model/applyStaticHooks.js index 934f9452ade..957e94f2288 100644 --- a/lib/helpers/model/applyStaticHooks.js +++ b/lib/helpers/model/applyStaticHooks.js @@ -1,6 +1,6 @@ 'use strict'; -const middlewareFunctions = require('../query/applyQueryMiddleware').middlewareFunctions; +const middlewareFunctions = require('../../constants').queryMiddlewareFunctions; const promiseOrCallback = require('../promiseOrCallback'); module.exports = function applyStaticHooks(model, hooks, statics) { diff --git a/lib/helpers/query/applyGlobalOption.js b/lib/helpers/query/applyGlobalOption.js index 8888e368b9e..e93fa73d492 100644 --- a/lib/helpers/query/applyGlobalOption.js +++ b/lib/helpers/query/applyGlobalOption.js @@ -2,12 +2,12 @@ const utils = require('../../utils'); -function applyGlobalMaxTimeMS(options, model) { - applyGlobalOption(options, model, 'maxTimeMS'); +function applyGlobalMaxTimeMS(options, connectionOptions, baseOptions) { + applyGlobalOption(options, connectionOptions, baseOptions, 'maxTimeMS'); } -function applyGlobalDiskUse(options, model) { - applyGlobalOption(options, model, 'allowDiskUse'); +function applyGlobalDiskUse(options, connectionOptions, baseOptions) { + applyGlobalOption(options, connectionOptions, baseOptions, 'allowDiskUse'); } module.exports = { @@ -16,14 +16,14 @@ module.exports = { }; -function applyGlobalOption(options, model, optionName) { +function applyGlobalOption(options, connectionOptions, baseOptions, optionName) { if (utils.hasUserDefinedProperty(options, optionName)) { return; } - if (utils.hasUserDefinedProperty(model.db.options, optionName)) { - options[optionName] = model.db.options[optionName]; - } else if (utils.hasUserDefinedProperty(model.base.options, optionName)) { - options[optionName] = model.base.options[optionName]; + if (utils.hasUserDefinedProperty(connectionOptions, optionName)) { + options[optionName] = connectionOptions[optionName]; + } else if (utils.hasUserDefinedProperty(baseOptions, optionName)) { + options[optionName] = baseOptions[optionName]; } } diff --git a/lib/helpers/query/applyQueryMiddleware.js b/lib/helpers/query/applyQueryMiddleware.js deleted file mode 100644 index b3cd8ee0163..00000000000 --- a/lib/helpers/query/applyQueryMiddleware.js +++ /dev/null @@ -1,54 +0,0 @@ -'use strict'; - -/*! - * ignore - */ - -module.exports = applyQueryMiddleware; - -const validOps = require('./validOps'); - -/*! - * ignore - */ - -applyQueryMiddleware.middlewareFunctions = validOps.concat([ - 'validate' -]); - -/** - * Apply query middleware - * - * @param {Query} Query constructor - * @param {Model} model - * @api private - */ - -function applyQueryMiddleware(Query, model) { - const queryMiddleware = model.schema.s.hooks.filter(hook => { - const contexts = _getContexts(hook); - if (hook.name === 'validate') { - return !!contexts.query; - } - if (hook.name === 'deleteOne' || hook.name === 'updateOne') { - return !!contexts.query || Object.keys(contexts).length === 0; - } - if (hook.query != null || hook.document != null) { - return !!hook.query; - } - return true; - }); - - Query.prototype._queryMiddleware = queryMiddleware; -} - -function _getContexts(hook) { - const ret = {}; - if (hook.hasOwnProperty('query')) { - ret.query = hook.query; - } - if (hook.hasOwnProperty('document')) { - ret.document = hook.document; - } - return ret; -} diff --git a/lib/helpers/query/castFilterPath.js b/lib/helpers/query/castFilterPath.js index dbd0109c915..c5c8d0fadfd 100644 --- a/lib/helpers/query/castFilterPath.js +++ b/lib/helpers/query/castFilterPath.js @@ -2,8 +2,7 @@ const isOperator = require('./isOperator'); -module.exports = function castFilterPath(query, schematype, val) { - const ctx = query; +module.exports = function castFilterPath(ctx, schematype, val) { const any$conditionals = Object.keys(val).some(isOperator); if (!any$conditionals) { diff --git a/lib/helpers/query/completeMany.js b/lib/helpers/query/completeMany.js deleted file mode 100644 index a27ecafa385..00000000000 --- a/lib/helpers/query/completeMany.js +++ /dev/null @@ -1,36 +0,0 @@ -'use strict'; - -const helpers = require('../../queryHelpers'); - -module.exports = completeMany; - -/** - * Given a model and an array of docs, hydrates all the docs to be instances - * of the model. Used to initialize docs returned from the db from `find()` - * - * @param {Model} model - * @param {Array} docs - * @param {Object} fields the projection used, including `select` from schemas - * @param {Object} userProvidedFields the user-specified projection - * @param {Object} [opts] - * @param {Array} [opts.populated] - * @param {ClientSession} [opts.session] - * @param {Function} callback - * @api private - */ - -async function completeMany(model, docs, fields, userProvidedFields, opts) { - return Promise.all(docs.map(doc => new Promise((resolve, reject) => { - const rawDoc = doc; - doc = helpers.createModel(model, doc, fields, userProvidedFields); - if (opts.session != null) { - doc.$session(opts.session); - } - doc.$init(rawDoc, opts, (err) => { - if (err != null) { - return reject(err); - } - resolve(doc); - }); - }))); -} diff --git a/lib/helpers/query/validOps.js b/lib/helpers/query/validOps.js index 2eb4375a93f..2d1aa477b1c 100644 --- a/lib/helpers/query/validOps.js +++ b/lib/helpers/query/validOps.js @@ -1,20 +1,3 @@ 'use strict'; -module.exports = Object.freeze([ - // Read - 'countDocuments', - 'distinct', - 'estimatedDocumentCount', - 'find', - 'findOne', - // Update - 'findOneAndReplace', - 'findOneAndUpdate', - 'replaceOne', - 'updateMany', - 'updateOne', - // Delete - 'deleteMany', - 'deleteOne', - 'findOneAndDelete' -]); +module.exports = require('../../constants').queryMiddlewareFunctions; diff --git a/lib/model.js b/lib/model.js index 1ffca2b1b6e..8795e2ac351 100644 --- a/lib/model.js +++ b/lib/model.js @@ -22,7 +22,6 @@ const VersionError = require('./error/version'); const ParallelSaveError = require('./error/parallelSave'); const applyDefaultsHelper = require('./helpers/document/applyDefaults'); const applyDefaultsToPOJO = require('./helpers/model/applyDefaultsToPOJO'); -const applyQueryMiddleware = require('./helpers/query/applyQueryMiddleware'); const applyHooks = require('./helpers/model/applyHooks'); const applyMethods = require('./helpers/model/applyMethods'); const applyProjection = require('./helpers/projection/applyProjection'); @@ -4765,7 +4764,7 @@ Model.compile = function compile(name, schema, collectionName, connection, base) Object.setPrototypeOf(model.Query.prototype, Query.prototype); model.Query.base = Query.base; model.Query.prototype.constructor = Query; - applyQueryMiddleware(model.Query, model); + model._applyQueryMiddleware(); applyQueryMethods(model, schema.query); return model; @@ -4874,6 +4873,41 @@ if (util.inspect.custom) { Model[util.inspect.custom] = Model.inspect; } +/*! + * Applies query middleware from this model's schema to this model's + * Query constructor. + */ + +Model._applyQueryMiddleware = function _applyQueryMiddleware() { + const Query = this.Query; + const queryMiddleware = this.schema.s.hooks.filter(hook => { + const contexts = _getContexts(hook); + if (hook.name === 'validate') { + return !!contexts.query; + } + if (hook.name === 'deleteOne' || hook.name === 'updateOne') { + return !!contexts.query || Object.keys(contexts).length === 0; + } + if (hook.query != null || hook.document != null) { + return !!hook.query; + } + return true; + }); + + Query.prototype._queryMiddleware = queryMiddleware; +}; + +function _getContexts(hook) { + const ret = {}; + if (hook.hasOwnProperty('query')) { + ret.query = hook.query; + } + if (hook.hasOwnProperty('document')) { + ret.document = hook.document; + } + return ret; +} + /*! * Module exports. */ diff --git a/lib/query.js b/lib/query.js index f94acc7f0a6..245c3d48d49 100644 --- a/lib/query.js +++ b/lib/query.js @@ -19,7 +19,6 @@ const castArrayFilters = require('./helpers/update/castArrayFilters'); const castNumber = require('./cast/number'); const castUpdate = require('./helpers/query/castUpdate'); const clone = require('./helpers/clone'); -const completeMany = require('./helpers/query/completeMany'); const getDiscriminatorByValue = require('./helpers/discriminator/getDiscriminatorByValue'); const helpers = require('./queryHelpers'); const immediate = require('./helpers/immediate'); @@ -40,7 +39,7 @@ const specialProperties = require('./helpers/specialProperties'); const updateValidators = require('./helpers/updateValidators'); const util = require('util'); const utils = require('./utils'); -const validOps = require('./helpers/query/validOps'); +const queryMiddlewareFunctions = require('./constants').queryMiddlewareFunctions; const queryOptionMethods = new Set([ 'allowDiskUse', @@ -457,7 +456,7 @@ Query.prototype.slice = function() { * ignore */ -const validOpsSet = new Set(validOps); +const validOpsSet = new Set(queryMiddlewareFunctions); Query.prototype._validateOp = function() { if (this.op != null && !validOpsSet.has(this.op)) { @@ -2233,8 +2232,8 @@ Query.prototype._find = async function _find() { const _this = this; const userProvidedFields = _this._userProvidedFields || {}; - applyGlobalMaxTimeMS(this.options, this.model); - applyGlobalDiskUse(this.options, this.model); + applyGlobalMaxTimeMS(this.options, this.model.db.options, this.model.base.options); + applyGlobalDiskUse(this.options, this.model.db.options, this.model.base.options); // Separate options to pass down to `completeMany()` in case we need to // set a session on the document @@ -2271,7 +2270,7 @@ Query.prototype._find = async function _find() { } return mongooseOptions.lean ? _completeManyLean(_this.model.schema, docs, null, completeManyOptions) : - completeMany(_this.model, docs, fields, userProvidedFields, completeManyOptions); + _this._completeMany(docs, fields, userProvidedFields, completeManyOptions); } const pop = helpers.preparePopulationOptionsMQ(_this, mongooseOptions); @@ -2279,7 +2278,7 @@ Query.prototype._find = async function _find() { return _this.model.populate(docs, pop); } - docs = await completeMany(_this.model, docs, fields, userProvidedFields, completeManyOptions); + docs = await _this._completeMany(docs, fields, userProvidedFields, completeManyOptions); await this.model.populate(docs, pop); return docs; @@ -2473,6 +2472,38 @@ Query.prototype._completeOne = function(doc, res, callback) { }); }; +/** + * Given a model and an array of docs, hydrates all the docs to be instances + * of the model. Used to initialize docs returned from the db from `find()` + * + * @param {Model} model + * @param {Array} docs + * @param {Object} fields the projection used, including `select` from schemas + * @param {Object} userProvidedFields the user-specified projection + * @param {Object} [opts] + * @param {Array} [opts.populated] + * @param {ClientSession} [opts.session] + * @param {Function} callback + * @api private + */ + +Query.prototype._completeMany = async function _completeMany(docs, fields, userProvidedFields, opts) { + const model = this.model; + return Promise.all(docs.map(doc => new Promise((resolve, reject) => { + const rawDoc = doc; + doc = helpers.createModel(model, doc, fields, userProvidedFields); + if (opts.session != null) { + doc.$session(opts.session); + } + doc.$init(rawDoc, opts, (err) => { + if (err != null) { + return reject(err); + } + resolve(doc); + }); + }))); +}; + /** * Internal helper to execute a findOne() operation * @@ -2488,8 +2519,8 @@ Query.prototype._findOne = async function _findOne() { throw err; } - applyGlobalMaxTimeMS(this.options, this.model); - applyGlobalDiskUse(this.options, this.model); + applyGlobalMaxTimeMS(this.options, this.model.db.options, this.model.base.options); + applyGlobalDiskUse(this.options, this.model.db.options, this.model.base.options); const options = this._optionsForExec(); @@ -2585,8 +2616,8 @@ Query.prototype._countDocuments = async function _countDocuments() { throw this.error(); } - applyGlobalMaxTimeMS(this.options, this.model); - applyGlobalDiskUse(this.options, this.model); + applyGlobalMaxTimeMS(this.options, this.model.db.options, this.model.base.options); + applyGlobalDiskUse(this.options, this.model.db.options, this.model.base.options); const options = this._optionsForExec(); @@ -2754,8 +2785,8 @@ Query.prototype.__distinct = async function __distinct() { throw this.error(); } - applyGlobalMaxTimeMS(this.options, this.model); - applyGlobalDiskUse(this.options, this.model); + applyGlobalMaxTimeMS(this.options, this.model.db.options, this.model.base.options); + applyGlobalDiskUse(this.options, this.model.db.options, this.model.base.options); const options = this._optionsForExec(); this._applyTranslateAliases(options); @@ -3247,8 +3278,8 @@ Query.prototype._findOneAndUpdate = async function _findOneAndUpdate() { throw this.error(); } - applyGlobalMaxTimeMS(this.options, this.model); - applyGlobalDiskUse(this.options, this.model); + applyGlobalMaxTimeMS(this.options, this.model.db.options, this.model.base.options); + applyGlobalDiskUse(this.options, this.model.db.options, this.model.base.options); if ('strict' in this.options) { this._mongooseOptions.strict = this.options.strict; diff --git a/lib/schema.js b/lib/schema.js index 6f25a00ccbe..b343fbce5af 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -29,8 +29,7 @@ const hasNumericSubpathRegex = /\.\d+(\.|$)/; let MongooseTypes; -const queryHooks = require('./helpers/query/applyQueryMiddleware'). - middlewareFunctions; +const queryHooks = require('./constants').queryMiddlewareFunctions; const documentHooks = require('./helpers/model/applyHooks').middlewareFunctions; const hookNames = queryHooks.concat(documentHooks). reduce((s, hook) => s.add(hook), new Set()); From c164d63fef75867e8d3b0add164b26191e9795f5 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 19 Feb 2024 16:29:43 -0500 Subject: [PATCH 2/3] Update lib/query.js Co-authored-by: hasezoey --- lib/query.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/query.js b/lib/query.js index 245c3d48d49..179b2924b5c 100644 --- a/lib/query.js +++ b/lib/query.js @@ -2476,7 +2476,6 @@ Query.prototype._completeOne = function(doc, res, callback) { * Given a model and an array of docs, hydrates all the docs to be instances * of the model. Used to initialize docs returned from the db from `find()` * - * @param {Model} model * @param {Array} docs * @param {Object} fields the projection used, including `select` from schemas * @param {Object} userProvidedFields the user-specified projection From 4876d1e6b77be92ebe4ddb78315113a6482ee01c Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 19 Feb 2024 16:29:48 -0500 Subject: [PATCH 3/3] Update lib/query.js Co-authored-by: hasezoey --- lib/query.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/query.js b/lib/query.js index 179b2924b5c..8731f003204 100644 --- a/lib/query.js +++ b/lib/query.js @@ -2482,7 +2482,6 @@ Query.prototype._completeOne = function(doc, res, callback) { * @param {Object} [opts] * @param {Array} [opts.populated] * @param {ClientSession} [opts.session] - * @param {Function} callback * @api private */