Create an adapter method to handle virtual population

[Freezystem]

Handle virtual population in a dedicated method of the mongoose adapter

I've separated, in a dedicated method, the code that handles the virtuals population. It allows the code to be more readable and easier to unit test.

Also I've included some short-circuits to avoid useless overhead and keep the virtual step as fast as possible, especially when it need to be bypassed.

Now, You can also disable the virtual population mechanism by adding {virtuals: false} in the service settings (request made by @thib3113). Note that letting the service decide by itself is now nearly as fast as adding this option.

Overridding virtual populate defaults

You can now override populate options for each virtuals separately by providing an object in the service settings.virtuals.

example:

{
  virtuals: {
    myVirtual: {
      select: "foo bar baz",
      options: { lean: false },
      transform: (doc) => doc,
    }
  }
}

Remove restricted field projection in virtual populate if there is a match clause

I recently came accross an issue involving virtuals with a match clause.

When I implemented the virtual population the first time, I intentionally limited the retrieved fields to the _id only to optimize memory/bandwith usage.
Unfortunately it has the side effect to prevent virtual match clauses from filtering properly.

Indeed, mongoose execute the match clause as a filter on the populated list but not in the populate query itself as I previously though.
Thus, when the virtual has a match clause, we need to retrieve the whole document to allow its proper execution.

I also allow the population of virtuals using the refPath option, which is not documented but allowed by the code. Previously only virtuals with ref option would have been populated. see code

[thib3113]

as I said, I think service.settings.virtuals need to be false by default (or checking if it's strictly true) .

Because, I think this library need to be "moleculer first" (so, by default use moleculer => propagate) . And if you use edge case you can enable it . ( @Freezystem I think you are in an edge case, because the documentation say this module use "one database per service pattern", and you didn't follow it )

Also, I think virtuals "objects" need to be populated too . Just, instead of doing a schema request, just map the _id from the field to the virtual .

[Freezystem]

I understand what you mean but I disagree in a sense that people using virtuals as I do will not understand why they don't populate by default.

I'm using virtuals with the full power of moleculer to call the proper action once they are populated with ids.

If you look closesly in the tests, I'm only populating virtuals _id by default in a lean query and then use the populate mechanism of moleculer to properly populate the ids with the corresponding service action.
The virtual step is only here to allow moleculer to post populate those documents.
Due to how mongoose and moleculer-db-adapter-mongoose works, it's the only way to allow virtuals to be post populated by moleculer otherwise they would always be empty.

So in my opinion, I'm not in an "edge" case at all. I'm using the mongoose full potential and the db adapter should let me handle that natively without any additional configuration.

We can argue about disabling it by default but it has no downside in your case as it is short-circuiting almost immediately even if you're not using the {virtuals: false} option explicitely.

[thib3113]

I understand what you mean but I disagree in a sense that people using virtuals as I do will not understand why they don't populate by default.

And in the opposite, people using virtuals and propagate will get a crash, and won't understand why it crash if they follow the documentation .

I think, only @icebob can decide . but in my opinion, moleculer-db and his adapters need to be "moleculer first" .

And, if you want to use mongoose at the full potential (and so don't follow the base concept), this is not a problem, and you have the option to do it .

If you look closesly in the tests, I'm only populating virtuals _id by default in a lean query and then use the populate mechanism of moleculer to properly populate the ids with the corresponding service action.

My opinion about it, is to directly populate it in the entity object . Without using populate, so we can propagate on it .

So, virtuals can be used ( a field like lastPostWithVotes can be done on the User, and propagate will trigger a action get this post, maybe with an action like post.lastUpvotedPosts {author:XXX}

About the rest, ok, so we didn't aggree ... so, the decision came to @icebob . Did the moleculer-db-adapter-mongoose need to be moleculer first (using propagate), or mongoose first (using internal mongoose queries)

[Freezystem]

Following your feedbacks I refactored the virtual population to make it moleculer first.

From now on, the default behaviour will be to map virtual fields to their localField key when they got one. Allowing the populate mechanism to pass the localField value to the desired service action afterward.

To use the native mongoose virtual population workflow, you'll need to set the useNativeMongooseVirtuals from your service settings to true.

Feel free to check the tests to see if the implementation I made fit your usage.

[thib3113]

Yes In my opinion, it can be good .

it allow a moleculer user to use mongoose virtuals, with "one database per service" . he just need to create the correct actions to propagate .

[icebob]

I do not fully understand every part of this virtual populating but the code looks good to me.

[Freezystem]

PR was missing a test. It's now ready for review and merge

[icebob]

There is a pointer to the service instace as this.service. Please use it instead of ctx.service...

In addition, this property is very constant, so better if you get it in init to a local class variable because lodash get is not too fast.

[Freezystem]

I removed unnecessary use of _.get, it should be ok now.