Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: memoize toJSON / toObject default options #14672

Merged
merged 3 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 10 additions & 17 deletions lib/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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 } : {};
Expand All @@ -3815,18 +3806,18 @@ 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;
options._seen = options._seen || new Map();

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
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 2 additions & 0 deletions lib/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -5105,6 +5105,8 @@ Model.recompileSchema = function recompileSchema() {
}
}

delete this.schema._defaultToObjectOptionsMap;

applyEmbeddedDiscriminators(this.schema, new WeakSet(), true);
};

Expand Down
24 changes: 24 additions & 0 deletions lib/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
21 changes: 10 additions & 11 deletions test/document.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -433,9 +424,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');
Expand All @@ -452,6 +444,7 @@ describe('document', function() {
return { myid: ret._id.toString() };
};

delete doc.schema._defaultToObjectOptionsMap;
clone = doc.toObject();
assert.deepEqual(out, clone);

Expand Down Expand Up @@ -489,6 +482,7 @@ describe('document', function() {

// all done
delete doc.schema.options.toObject;
delete doc.schema._defaultToObjectOptionsMap;
});

it('toObject transform', async function() {
Expand Down Expand Up @@ -884,6 +878,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);
Expand All @@ -897,6 +892,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);
Expand Down Expand Up @@ -932,6 +928,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);
Expand All @@ -951,6 +948,7 @@ describe('document', function() {
return { myid: ret._id.toString() };
};

delete doc.schema._defaultToObjectOptionsMap;
clone = doc.toJSON();
assert.deepEqual(out, clone);

Expand Down Expand Up @@ -988,6 +986,7 @@ describe('document', function() {

// all done
delete doc.schema.options.toJSON;
delete doc.schema._defaultToObjectOptionsMap;
});

it('jsonifying an object', function() {
Expand All @@ -998,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);
});

Expand Down
8 changes: 3 additions & 5 deletions test/document.unit.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

'use strict';

const Schema = require('../lib/schema');
const start = require('./common');

const assert = require('assert');
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand Down
8 changes: 7 additions & 1 deletion test/model.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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() {
Expand Down