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

Hydrate does not call setters on the schema #11653

Closed
zxlin opened this issue Apr 9, 2022 · 6 comments
Closed

Hydrate does not call setters on the schema #11653

zxlin opened this issue Apr 9, 2022 · 6 comments
Labels
enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature
Milestone

Comments

@zxlin
Copy link

zxlin commented Apr 9, 2022

Bug report

Steps to Reproduce

Example closely modeled from docs on setters: https://mongoosejs.com/docs/tutorials/getters-setters.html#setters

const userSchema = new mongoose.Schema({
  email: {
    type: String,
    set: v => v.toLowerCase(), // this should be called by hydrate
  }
});

const User = mongoose.model('User', userSchema);

const object = { email: 'TEST@gmail.com' };

const user = User.hydrate(object);

console.log(user); // => { email: 'TEST@gmail.com' }

Expected Behavior

console.log(user); // should yield => { email: 'test@gmail.com' }

Basically the below 2 functions should yield the same thing, but it currently does not

console.log((new User(object).email); // => test@gmail.com
console.log((User.hydrate(object)).email); // => TEST@gmail.com

Versions

mongoose@6.2.10
Node v14.19.1
MongoDB: 4.4.6

Possible Resolutions

I understand why setters might not be called, if it was truly hydrating, then perhaps the setter is redundant, but in the case where a schema field may have a setter and getter, this does become necessary. So option 1 is to call the setter always, option 2 is a parameter option to enable calling the setter. I'm not sure what is the best approach, wanted to get some feedback here.

Thanks for the great work everyone!

@IslandRhythms IslandRhythms added the confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. label Apr 11, 2022
@vkarpov15 vkarpov15 added this to the 6.2.15 milestone Apr 14, 2022
@vkarpov15
Copy link
Collaborator

This is currently expected behavior. hydrate() is meant to hydrate a document that is loaded from MongoDB, so the assumption is that setters already ran.

We can add an option to hydrate() to allow running setters. But I'm curious what your use case is - why use User.hydrate() instead of new User()?

@vkarpov15 vkarpov15 added enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature and removed confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. labels May 6, 2022
@vkarpov15 vkarpov15 modified the milestones: 6.3.4, 6.x Unprioritized May 6, 2022
@zxlin
Copy link
Author

zxlin commented May 7, 2022

@vkarpov15 thanks for the reply! We're currently caching an array of mongoose objects, however, the storage mechanism only allows for plain old JavaScript objects. So we want to hydrate the POJOs when we need to use it and ensure that the mongoose object isn't treated as a "new" object hence we don't use the new constructor.

@vkarpov15
Copy link
Collaborator

Where do the POJOs in your cache come from?

@zxlin
Copy link
Author

zxlin commented May 27, 2022

Hi @vkarpov15, the POJOs are from mongoose documents, the result of .toObject()

@vkarpov15
Copy link
Collaborator

@zxlin so those documents should have setters run on them at some point, at least when they're created. One alternative approach is to call const doc = new Model(obj); to create a new model instance and run setters before calling toObject(). Does that work for you?

@zxlin
Copy link
Author

zxlin commented Jun 7, 2022

Hi @vkarpov15, so the flow is something like the below, and let's assume we have a field called text and the model has this schema on the text field:

text: {
  type: String,
  set: (v) => v.toLowerCase(),
  get : (v) => v.toUpperCase(),
}

So the steps:

  1. retrieve document from mongodb: const doc = await Model.findOne({ text: 'abc' }) (here doc.text === 'abc')
  2. convert to POJO: const obj = doc.toObject() (at this point, obj.text === 'ABC' due to the getter)
  3. store obj into cache
  4. retrieve obj from cache
  5. hydrate the POJO: const doc = Model.hydrate(obj)
  6. here is where we run into the issue: while obj has the text field, because .hydrate doesn't call the setter, that field is not set correctly and has the wrong value. In this case, doc.text is upper case 'ABC' which is incorrect and does not match our step 1 which is the expected value of lower case 'abc'

This is the simplified example, hopefully this helps you better understand where we're encountering this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature
Projects
None yet
Development

No branches or pull requests

3 participants