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

getters array in virtualtype for "id" virtual is not deduped #14457

Closed
2 tasks done
kmonagle opened this issue Mar 21, 2024 · 1 comment · Fixed by #14492
Closed
2 tasks done

getters array in virtualtype for "id" virtual is not deduped #14457

kmonagle opened this issue Mar 21, 2024 · 1 comment · Fixed by #14492
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Milestone

Comments

@kmonagle
Copy link

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Mongoose version

7.4.0

Node.js version

14.x

MongoDB server version

4.4

Typescript version (if applicable)

No response

Description

In Mongoose 5 the getters array for the "id" virtual was deduped. In Mongoose 6+ this no longer happens.

In Mongoose 6+, If models are repeatedly recreated, a new idGetter is added to the getters array with each model recreation. In my app, after a load test, the "id" getters array can grow to thousands of redundant idGetter functions. We use lean virtuals, so those thousands of functions are executed for every new object, and performance is severely degraded after a while.

Use case: we hot load and dynamically recreate models very frequently in our app.

In Mongoose 5 when a model was compiled there was a check to ensure that redundant functions didn't get added to the "id" virtual getter array:

Document.prototype.$__setSchema = function(schema) {
  schema.plugin(idGetter, { deduplicate: true });
Schema.prototype.plugin = function(fn, opts) {
  if (typeof fn !== 'function') {
    throw new Error('First param to `schema.plugin()` must be a function, ' +
      'got "' + (typeof fn) + '"');
  }

  if (opts && opts.deduplicate) {
    for (const plugin of this.plugins) {
      if (plugin.fn === fn) {
        return this;
      }
    }
  }
  this.plugins.push({ fn: fn, opts: opts });

  fn(this, opts);
  return this;
};

This check does not appear to exist in 6+, so the array can grow dramatically with redundant functions when models are regularly recreated.

Steps to Reproduce

const mongoose = require("mongoose")

const nameSchema = new mongoose.Schema({"name" : {
  "type" : "String"
}});

const Name = mongoose.model('Name', nameSchema);
console.log(mongoose.models);


for(let i = 0;i<10;i++){
  mongoose.model('Name', nameSchema);
  delete mongoose.models['Name'];
}

I added some console output in the mongoose source code, Virtualtype get method:

VirtualType.prototype.get = function(fn) {
  this.getters.push(fn);
  console.log("virtual path: ", this.path);
  console.log("getters size: ", this.getters.length);
  console.log("getters: ", this.getters);
  return this;
};

The output of the last few iterations of the loop in the script above show the "id" getters array growing with duplicate idGetter functions:

virtual path:  id
getters size:  8
getters:  [
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter]
]
autoIdGetter:  true
adding ID getter 
virtual path:  id
getters size:  9
getters:  [
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter]
]
autoIdGetter:  true
adding ID getter 
virtual path:  id
getters size:  10
getters:  [
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter],
  [Function: idGetter]
]

Expected Behavior

I'd expect Mongoose to do a 5.x-type sanity check to prevent duplicate functions from proliferating in virtual getter arrays. Something like this would fix the issue:

VirtualType.prototype.get = function(fn) {

  for (const getter of this.getters) {
     if (getter === fn) {
       return this;
     }
  } 

  this.getters.push(fn);
  return this;
};
@IslandRhythms IslandRhythms added the confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. label Mar 22, 2024
@IslandRhythms
Copy link
Collaborator

const mongoose = require('mongoose');

const nameSchema = new mongoose.Schema({"name" : {
  "type" : "String"
}});

const Name = mongoose.model('Name', nameSchema);
console.log(mongoose.models);
console.log(Name.schema.virtuals)

for(let i = 0;i<10;i++){
  mongoose.model('Name', nameSchema);
  delete mongoose.models['Name'];
}

console.log(Name.schema.virtuals);

@vkarpov15 vkarpov15 added this to the 7.6.11 milestone Mar 29, 2024
vkarpov15 added a commit that referenced this issue Apr 4, 2024
fix(schema): deduplicate idGetter so creating multiple models with same schema doesn't result in multiple id getters
vkarpov15 added a commit that referenced this issue Apr 4, 2024
…me schema doesn't result in multiple id getters

Fix #14457
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Projects
None yet
3 participants