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

When calling lean() with transform on a document with embedded schema, the transform is also run on every level of embedded documents instead of just the parent document #13757

Closed
2 tasks done
sharang-lyric opened this issue Aug 21, 2023 · 10 comments
Labels
docs This issue is due to a mistake or omission in the mongoosejs.com documentation
Milestone

Comments

@sharang-lyric
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.3

Node.js version

18.16.1

MongoDB server version

5.0

Typescript version (if applicable)

No response

Description

I have a model Store with an array property employees where each element is a nested schema EmployeeSchema.

When calling Store.findOne() with a lean transform function, in addition to being called on the store object, it is also being called on:

  1. The employees array itself
  2. Every employee within the employees array

Steps to Reproduce

Code:

import mongoose from 'mongoose';

const shiftSchema = new mongoose.Schema({
  employeeId: mongoose.Types.ObjectId,
  startedAt: Date,
  endedAt: Date
});

const Shift = mongoose.model('Shift', shiftSchema);

const employeeSchema = new mongoose.Schema({
  name: String,
});

const storeSchema = new mongoose.Schema({
  location: String,
  employees: [employeeSchema],
});

const Store = mongoose.model('Store', storeSchema)

mongoose.connect(process.env.MONGO_URI);

await Store.create({
  location: 'Tashbaan',
  employees: [
    { name: 'Aravis' },
    { name: 'Shasta' },
  ],
});

const findStoreResult = await Store.findOne({ location: 'Tashbaan' })
  .lean({
    transform (doc) {
      doc.transformed = true;
      return doc;
    },
  })
  .exec();

console.log(findStoreResult);

await Shift.deleteMany({});
await Store.deleteMany({});

Output:

{
  _id: new ObjectId("64e2dca6901fb4810d525d3f"),
  location: 'Tashbaan',
  employees: [
    {
      name: 'Aravis',
      _id: new ObjectId("64e2dca6901fb4810d525d40"),
      transformed: true
    },
    {
      name: 'Shasta',
      _id: new ObjectId("64e2dca6901fb4810d525d41"),
      transformed: true
    },
    transformed: true
  ],
  __v: 0,
  transformed: true
}

Expected Behavior

Only the outer store object should have transformed: true
Expected output:

{
  _id: new ObjectId("64e2dca6901fb4810d525d3f"),
  location: 'Tashbaan',
  employees: [
    {
      name: 'Aravis',
      _id: new ObjectId("64e2dca6901fb4810d525d40"),
    },
    {
      name: 'Shasta',
      _id: new ObjectId("64e2dca6901fb4810d525d41"),
    },
  ],
  __v: 0,
  transformed: true
}
@vkarpov15 vkarpov15 added this to the 7.4.5 milestone Aug 22, 2023
@vkarpov15 vkarpov15 added the has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue label Aug 22, 2023
@vkarpov15 vkarpov15 modified the milestones: 7.4.5, 7.5.1 Aug 25, 2023
@vkarpov15
Copy link
Collaborator

This is currently expected behavior: on toObject(), toJSON(), and lean(), setting transform to a function calls that transform on all subdocuments. Unfortunately we do not currently have a good workaround for configuring this behavior, but we will work on adding a way to avoid running lean transforms on all subdocs.

@vkarpov15 vkarpov15 added confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature and removed has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. labels Aug 28, 2023
@vkarpov15
Copy link
Collaborator

vkarpov15 commented Sep 4, 2023

On second thought, if you just want your transform logic to run on the top level document, you can do the following:

const findStoreResult = await Store.findOne({ location: 'Tashbaan' })
  .lean()
  .exec();
findStoreResult.transformed = true;

transform is useful specifically because it handles recursion on subdocuments. If you don't want that recursion, you can just execute the logic after the query is done. Does that work for you @sharang-lyric ?

@vkarpov15 vkarpov15 removed this from the 7.5.1 milestone Sep 4, 2023
@vkarpov15 vkarpov15 added needs clarification This issue doesn't have enough information to be actionable. Close after 14 days of inactivity and removed enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature labels Sep 4, 2023
@sharang-lyric
Copy link
Author

@vkarpov15 Of course, I could set the transform logic inside toObject() and just run toObject() after the query is done, but I specifically wanted to leverage the performance benefits that lean() offers.

To your point on handling recursion on subdocuments, there are other (IMO more appropriate, design-wise) ways to transform subdocuments:

  1. Explicitly perform subdocument transformations in the transform function
  2. Specify a separate lean transform when calling populate() for the subdocument

I think ideally, lean should be a schema configuration property similar to toObject and toJSON, but that might be for a separate discussion.

@vkarpov15
Copy link
Collaborator

My suggestion wasn't to remove lean() entirely. My suggestion is just to apply your transformation logic to the result of your lean query.

@sharang-lyric
Copy link
Author

@vkarpov15 Understood. I recently found that mongoose also allows me to set the lean transform in the schema itself, and have the transform run by default whenever I call lean(). This way I only have to define my transform function once. The only issue is the one in this thread where the transform runs on all subdocuments and subdocument arrays. If changed to only transform the document itself, this would be extremely helpful.

@vkarpov15
Copy link
Collaborator

@sharang-lyric unfortunately I don't see a valid reason to support running the transform on just the document. The logic from this comment is a simpler alternative that doesn't require any extra options. I'm going to close this issue for now.

@bazineta
Copy link

bazineta commented Feb 8, 2024

Apologies in advance for the necropost here, but I'm very curious. In my own usage, nested subdocuments are typically something with a containment relationship to the parent; that's it, that's all. They don't look like the parent, and the parent logic doesn't apply to them; they're contained entities.

It therefore seems a bit unusual that calling thing.toObject({ transform }) would by default recursively apply the same transform to any contained subdocuments, and I'm curious as to the use case.

The reason I ask, is that this exists in the documentation for toObject().

With transformations we can do a lot more than remove properties.
We can even return completely new customized objects:

I believe that in the absence of subdocuments, that's a valid statement, but in the presence of them, it becomes a trap for the unwary. I surmise that the recursive behavior is intended to support a use case of I want to delete _id fields from the top-level document and any subdocuments, or something of that nature. However, if your intent is instead to return a new completely customized object as the documentation describes, that's probably not going to end well when the subdocuments are run through the transform that was intended for the top-level document.

@vkarpov15
Copy link
Collaborator

Our reasoning is that it is much easier to just manipulate the return value of toObject() rather than using transform if you don't want the recursive behavior. The recursive behavior has its uses, like, as you mentioned, removing id properties.

@bazineta
Copy link

Fair enough; I suggest something more explicit in the documentation to that effect, i.e., when transforming an object, manipulating the return value of toObject() is in general going to be what you want to do, and using the transform option is going to be, except in relatively rare cases, probably not what you want to do.

@vkarpov15
Copy link
Collaborator

That's a reasonable suggestion, we'll do that.

@vkarpov15 vkarpov15 reopened this Feb 13, 2024
@vkarpov15 vkarpov15 added this to the 8.1.3 milestone Feb 13, 2024
@vkarpov15 vkarpov15 added docs This issue is due to a mistake or omission in the mongoosejs.com documentation and removed needs clarification This issue doesn't have enough information to be actionable. Close after 14 days of inactivity labels Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This issue is due to a mistake or omission in the mongoosejs.com documentation
Projects
None yet
Development

No branches or pull requests

3 participants