From ab48710c8271e100e86454461efede60d0096250 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 14 Jun 2024 11:39:58 -0400 Subject: [PATCH 1/2] perf(document): memoize getting schema defaultOptions in toObject re: #14394 --- lib/document.js | 27 ++++++++++----------------- lib/schema.js | 24 ++++++++++++++++++++++++ test/document.test.js | 8 +++++++- test/document.unit.test.js | 8 +++----- test/index.test.js | 6 +++--- 5 files changed, 47 insertions(+), 26 deletions(-) diff --git a/lib/document.js b/lib/document.js index bdb6e2e57df..75852422d5c 100644 --- a/lib/document.js +++ b/lib/document.js @@ -22,7 +22,6 @@ const clone = require('./helpers/clone'); const compile = require('./helpers/document/compile').compile; const defineKey = require('./helpers/document/compile').defineKey; const flatten = require('./helpers/common').flatten; -const get = require('./helpers/get'); const getEmbeddedDiscriminatorPath = require('./helpers/document/getEmbeddedDiscriminatorPath'); const getKeysInSchemaOrder = require('./helpers/schema/getKeysInSchemaOrder'); const getSubdocumentStrictValue = require('./helpers/schema/getSubdocumentStrictValue'); @@ -3798,15 +3797,7 @@ Document.prototype.$__handleReject = function handleReject(err) { */ Document.prototype.$toObject = function(options, json) { - const path = json ? 'toJSON' : 'toObject'; - const baseOptions = this.constructor && - this.constructor.base && - this.constructor.base.options && - get(this.constructor.base.options, path) || {}; - const schemaOptions = this.$__schema && this.$__schema.options || {}; - // merge base default options with Schema's set default options if available. - // `clone` is necessary here because `utils.options` directly modifies the second input. - const defaultOptions = Object.assign({}, baseOptions, schemaOptions[path]); + const defaultOptions = this.$__schema._defaultToObjectOptions(json); // If options do not exist or is not an object, set it to empty object options = utils.isPOJO(options) ? { ...options } : {}; @@ -3815,10 +3806,10 @@ Document.prototype.$toObject = function(options, json) { let _minimize; if (options._calledWithOptions.minimize != null) { _minimize = options.minimize; - } else if (defaultOptions.minimize != null) { + } else if (defaultOptions != null && defaultOptions.minimize != null) { _minimize = defaultOptions.minimize; } else { - _minimize = schemaOptions.minimize; + _minimize = this.$__schema.options.minimize; } options.minimize = _minimize; @@ -3826,7 +3817,7 @@ Document.prototype.$toObject = function(options, json) { const depopulate = options._calledWithOptions.depopulate ?? options._parentOptions?.depopulate - ?? defaultOptions.depopulate + ?? defaultOptions?.depopulate ?? false; // _isNested will only be true if this is not the top level document, we // should never depopulate the top-level document @@ -3835,9 +3826,11 @@ Document.prototype.$toObject = function(options, json) { } // merge default options with input options. - for (const key of Object.keys(defaultOptions)) { - if (options[key] == null) { - options[key] = defaultOptions[key]; + if (defaultOptions != null) { + for (const key of Object.keys(defaultOptions)) { + if (options[key] == null) { + options[key] = defaultOptions[key]; + } } } options._isNested = true; @@ -4118,10 +4111,10 @@ function applyVirtuals(self, json, options, toObjectOptions) { } if (assignPath.indexOf('.') === -1 && assignPath === path) { v = self.get(path, null, { noDottedPath: true }); - v = clone(v, options); if (v === void 0) { continue; } + v = clone(v, options); json[assignPath] = v; continue; } diff --git a/lib/schema.js b/lib/schema.js index 4dfefccf545..bb3480088c6 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -643,6 +643,30 @@ Schema.prototype.discriminator = function(name, schema, options) { return this; }; +/*! + * Get this schema's default toObject/toJSON options, including Mongoose global + * options. + */ + +Schema.prototype._defaultToObjectOptions = function(json) { + const path = json ? 'toJSON' : 'toObject'; + if (this._defaultToObjectOptionsMap && this._defaultToObjectOptionsMap[path]) { + return this._defaultToObjectOptionsMap[path]; + } + + const baseOptions = this.base && + this.base.options && + this.base.options[path] || {}; + const schemaOptions = this.options[path] || {}; + // merge base default options with Schema's set default options if available. + // `clone` is necessary here because `utils.options` directly modifies the second input. + const defaultOptions = Object.assign({}, baseOptions, schemaOptions); + + this._defaultToObjectOptionsMap = this._defaultToObjectOptionsMap || {}; + this._defaultToObjectOptionsMap[path] = defaultOptions; + return defaultOptions; +}; + /** * Adds key path / schema type pairs to this schema. * diff --git a/test/document.test.js b/test/document.test.js index e5dcfa3bb1a..d27e1dd6795 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -433,9 +433,10 @@ describe('document', function() { delete ret.oids; ret._id = ret._id.toString(); }; + delete doc.schema._defaultToObjectOptionsMap; clone = doc.toObject(); assert.equal(doc.id, clone._id); - assert.ok(undefined === clone.em); + assert.strictEqual(clone.em, undefined); assert.ok(undefined === clone.numbers); assert.ok(undefined === clone.oids); assert.equal(clone.test, 'test'); @@ -452,6 +453,7 @@ describe('document', function() { return { myid: ret._id.toString() }; }; + delete doc.schema._defaultToObjectOptionsMap; clone = doc.toObject(); assert.deepEqual(out, clone); @@ -884,6 +886,7 @@ describe('document', function() { }; doc.schema.options.toJSON = { virtuals: true }; + delete doc.schema._defaultToObjectOptionsMap; let clone = doc.toJSON(); assert.equal(clone.test, 'test'); assert.ok(clone.oids instanceof Array); @@ -897,6 +900,7 @@ describe('document', function() { delete path.casterConstructor.prototype.toJSON; doc.schema.options.toJSON = { minimize: false }; + delete doc.schema._defaultToObjectOptionsMap; clone = doc.toJSON(); assert.equal(clone.nested2.constructor.name, 'Object'); assert.equal(Object.keys(clone.nested2).length, 1); @@ -932,6 +936,7 @@ describe('document', function() { ret._id = ret._id.toString(); }; + delete doc.schema._defaultToObjectOptionsMap; clone = doc.toJSON(); assert.equal(clone._id, doc.id); assert.ok(undefined === clone.em); @@ -951,6 +956,7 @@ describe('document', function() { return { myid: ret._id.toString() }; }; + delete doc.schema._defaultToObjectOptionsMap; clone = doc.toJSON(); assert.deepEqual(out, clone); diff --git a/test/document.unit.test.js b/test/document.unit.test.js index dbae16908ac..eee6954c7b3 100644 --- a/test/document.unit.test.js +++ b/test/document.unit.test.js @@ -4,6 +4,7 @@ 'use strict'; +const Schema = require('../lib/schema'); const start = require('./common'); const assert = require('assert'); @@ -37,12 +38,9 @@ describe('toObject()', function() { beforeEach(function() { Stub = function() { - const schema = this.$__schema = { - options: { toObject: { minimize: false, virtuals: true } }, - virtuals: { virtual: 'test' } - }; + this.$__schema = new Schema({}, { toObject: { minimize: false, virtuals: true } }); + this.$__schema.virtual('virtual').get(function() { return 'test'; }); this._doc = { empty: {} }; - this.get = function(path) { return schema.virtuals[path]; }; this.$__ = {}; }; Stub.prototype = Object.create(mongoose.Document.prototype); diff --git a/test/index.test.js b/test/index.test.js index 27f975a9d21..4eccc90a206 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -215,7 +215,7 @@ describe('mongoose module:', function() { mongoose.set('toJSON', { virtuals: true }); - const schema = new Schema({}); + const schema = new mongoose.Schema({}); schema.virtual('foo').get(() => 42); const M = mongoose.model('Test', schema); @@ -225,7 +225,7 @@ describe('mongoose module:', function() { assert.equal(doc.toJSON({ virtuals: false }).foo, void 0); - const schema2 = new Schema({}, { toJSON: { virtuals: true } }); + const schema2 = new mongoose.Schema({}, { toJSON: { virtuals: true } }); schema2.virtual('foo').get(() => 'bar'); const M2 = mongoose.model('Test2', schema2); @@ -239,7 +239,7 @@ describe('mongoose module:', function() { mongoose.set('toObject', { virtuals: true }); - const schema = new Schema({}); + const schema = new mongoose.Schema({}); schema.virtual('foo').get(() => 42); const M = mongoose.model('Test', schema); From 4f645ed4fd8c85dc01f1c0fd8e94a222264bb518 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 17 Jun 2024 14:08:36 -0400 Subject: [PATCH 2/2] fix: make recompileSchema() also clear memoized toObject and toJSON default options --- lib/model.js | 2 ++ test/document.test.js | 13 +++---------- test/model.test.js | 8 +++++++- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/lib/model.js b/lib/model.js index a94e3caff64..d600bb67122 100644 --- a/lib/model.js +++ b/lib/model.js @@ -5105,6 +5105,8 @@ Model.recompileSchema = function recompileSchema() { } } + delete this.schema._defaultToObjectOptionsMap; + applyEmbeddedDiscriminators(this.schema, new WeakSet(), true); }; diff --git a/test/document.test.js b/test/document.test.js index d27e1dd6795..7c63eaee29d 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -100,15 +100,6 @@ schema.path('date').set(function(v) { return v; }); -/** - * Method subject to hooks. Simply fires the callback once the hooks are - * executed. - */ - -TestDocument.prototype.hooksTest = function(fn) { - fn(null, arguments); -}; - const childSchema = new Schema({ counter: Number }); const parentSchema = new Schema({ @@ -491,6 +482,7 @@ describe('document', function() { // all done delete doc.schema.options.toObject; + delete doc.schema._defaultToObjectOptionsMap; }); it('toObject transform', async function() { @@ -994,6 +986,7 @@ describe('document', function() { // all done delete doc.schema.options.toJSON; + delete doc.schema._defaultToObjectOptionsMap; }); it('jsonifying an object', function() { @@ -1004,7 +997,7 @@ describe('document', function() { // parse again const obj = JSON.parse(json); - assert.equal(obj.test, 'woot'); + assert.equal(obj.test, 'woot', JSON.stringify(obj)); assert.equal(obj._id, oidString); }); diff --git a/test/model.test.js b/test/model.test.js index d5937654a45..8b5e8555e2b 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -7445,7 +7445,7 @@ describe('Model', function() { }); it('supports recompiling model with new schema additions (gh-14296)', function() { - const schema = new mongoose.Schema({ field: String }); + const schema = new mongoose.Schema({ field: String }, { toObject: { virtuals: false } }); const TestModel = db.model('Test', schema); TestModel.schema.virtual('myVirtual').get(function() { return this.field + ' from myVirtual'; @@ -7455,6 +7455,12 @@ describe('Model', function() { TestModel.recompileSchema(); assert.equal(doc.myVirtual, 'Hello from myVirtual'); + assert.strictEqual(doc.toObject().myVirtual, undefined); + + doc.schema.options.toObject.virtuals = true; + TestModel.recompileSchema(); + assert.equal(doc.myVirtual, 'Hello from myVirtual'); + assert.equal(doc.toObject().myVirtual, 'Hello from myVirtual'); }); it('supports recompiling model with new discriminators (gh-14444) (gh-14296)', function() {