Querying related models make unnecessary requests

[regevbr]

Steps to reproduce

Create a model called Book with the properties id,author,title Add a hasMany relation between the default User model and the Book model using a through Model called UserToBook which has the following properties: userId,bookId

The relation is defined in the User model like so.

    "books": {
      "type": "hasMany",
      "model": "Book",
      "foreignKey": "userId",
      "through": "UserToBook",
      "keyThrough": "bookId"
    },

Current Behavior

Querying the user for his books using a where filter performs 2 requests - one to the through model and one to the actual related model. The query to the through model is adding an include to the related model, which causes a fetch of a lot of data from the related model. The reason this data is needed, is to extract the related model ids, so we can use it to filter out when performing the actual call the related model.

Expected Behavior

There is no need at all to fetch the related model data in the first call - we can simply get the ids from the through model, and use them

Link to reproduction sandbox

Working on it

Additional information

Since I'm using (sadly) loopback 2.x I have added the fix to my own fork - https://github.com/regevbr/loopback-datasource-juggler/blob/2.x/lib/scope.js You can understand and base the fix based on the changes there

https://github.com/regevbr/loopback-datasource-juggler/blob/2cf8269e41a153d01e4d505af586bad9bb9110dd/lib/scope.js#L100-L138

[dhmlau]

@regevbr, would having a custom scope address your issue? See https://github.com/strongloop/loopback-next/issues/3453.

[regevbr]

Hi @dhmlau sadly I'm still using loopback v2. Also, I don't see how a custom scope will help here. Can you please elaborate

[dhmlau]

@regevbr, from my understanding, setting the custom scope will allow you to pick certain properties of the related models to be returned. But the above GH issue is for LB4. Sorry that I didn't notice you're using LB2. Would you consider updating the LoopBack version, at least to LB3? Migrating from LB2.x to LB3.x should be relatively straightforward: https://loopback.io/doc/en/lb3/Migrating-to-3.0.html.

[regevbr]

@dhmlau thanks for the info. Sadly I don't have the times or means to do it (i won't elaborate how hard it will be for me, but believe me I want to) In any case I fixed it for myself, just wanted it to be fixed in LB3, so when I upgrade, I won't have any issuesd

[bajtos]

Thank you @regevbr for reporting the issue. I am little bit confused about the fix shown in https://github.com/regevbr/loopback-datasource-juggler/blob/2cf8269e41a153d01e4d505af586bad9bb9110dd/lib/scope.js#L100-L138, it seems to add another database query to the path.

It would be best if you could create a small app reproducing the problem, so that we can better understand what you are trying to achieve and work together to find the best solution.

In any case I fixed it for myself, just wanted it to be fixed in LB3, so when I upgrade, I won't have any issues

That's a great plan, let's work together to make it happen :)

[regevbr]

@bajtos the actual changes I made there are depicted at https://github.com/regevbr/loopback-datasource-juggler/commit/2cf8269e41a153d01e4d505af586bad9bb9110dd#diff-c2a679b29102d5574291e884531cb8aeL135 I didn't add the extra query, it was always there and should be there, I just removed the inclusion of the related model from when querying the through model, as the inclusion has no point to it... I will try to find the time to create a reproduction repo, but it is important we will be on the same page as to what the issue is.

[bajtos]

Thank you for the link to the commit, I think I better understand your use case. If you can add (unit) tests to your commit and contribute the changes in a pull request, then it's not necessary to build a reproduction repo. Our current test suite for hasAndBelongsToMany starts here:

https://github.com/strongloop/loopback-datasource-juggler/blob/770f11b3a618ac879b6a6d021b109ecaed1aa1a2/test/relations.test.js#L4138

To me, it's important for the tests to verify that even when include is removed from the query executed against the "through" model (UserToBook in your case), we still honor include.scope when querying the target model (Book).