Skip to content

Commit

Permalink
Merge pull request #14672 from Automattic/vkarpov15/gh-14394-memoize-…
Browse files Browse the repository at this point in the history
…defaultoptions

perf: memoize toJSON / toObject default options
  • Loading branch information
vkarpov15 committed Jun 18, 2024
2 parents 174d8be + 4f645ed commit 0502d2b
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 37 deletions.
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

0 comments on commit 0502d2b

Please sign in to comment.