sails-hook-sequelize-blueprints icon indicating copy to clipboard operation
sails-hook-sequelize-blueprints copied to clipboard

Problem with populate

Open ybrodsky opened this issue 8 years ago • 6 comments

There's a problem in the way you are parsing the populate option.

Sequelize lets you do cool stuff as follows:

//A query to be run on MyMainModel
include: [
  {
    model: MyModel,
    attributes: ['id', 'name'],
    include: [{model: MyModelTwo}]
  }
]

With this query in sequelize I would be retrieving MyMainModel which contains MyModel which contains MyModelTwo.

The way you are parsing the populate param expects either an string or an array of strings (same as waterline) and with that creates a simple include (same as waterline) without the chance to do multiple joins and conditions in your joins, etc.

Anyway, this is important. I am trying to move from waterline because it basically sucks, and with this you are not using all the cool features that Sequelize has.

ybrodsky avatar May 06 '16 16:05 ybrodsky

Hi, A question, you're talking about the url populate option https://localhost:1337/MyMainModel?populate=[MyModel] or on javascript query itself?

cesardeazevedo avatar May 10 '16 02:05 cesardeazevedo

Well, its blueprints. I am talking about the url option and the way you parse it in your actionsUtil.

Thanks.

ybrodsky avatar May 10 '16 03:05 ybrodsky

This hooks is actually a complete fork from the original sails blueprints, which only allows only two levels deep, so, i don't think it's actually a problem, its just sails blueprints normal behaviour.

And you can still write an action on controller to handle these nested includes very easily.

About the &populate=[MyModel] param, i've to agree, it's very limited, but i don't see how it could be improved to support nested associations without a hard fork effort implementation and having a very ugly query url to support &limit=, &where=, &sort= for all child's associations and... RECURSIVELY.

I'd love to add these sequelize features without having to do hard changes like that, and not moving away from the original, which has been the original goal of this hook so far, it seems that is too much effort for just one specifically use case, which you can get several ways around to achieve basically the same result in different ways, even with /MyMainModel/1/MyModel?where={}&limit=30, MyModel/1 MyModelTwo?where={} or write any query on controller.

I would like that you share your concerns about it, so we can achieve a widespread consensus, also pull-requests are very appreciated.

cesardeazevedo avatar May 10 '16 09:05 cesardeazevedo

Well, but Sails limitation comes from Waterline, they don't let you do nested populate. For example, I have a small destinations api with the following:

City belongsTo Country belongsTo Region

It's quite common that I want to retrieve a City with it's Country and it's Region. To put it into MySQL, I want to do this:

SELECT * FROM `cities` 
LEFT JOIN countries on cities.country_id = countries.id 
LEFT JOIN regions on regions.id = countries.region_id
WHERE cities.id = 1

With Waterline I can't do that, I have to do two queries: fetch a City, populate the Country and then fetch the region or fetch the City and then fetch the Country and populate the Region.

This is a simple example, imagine if you have several models with complex nested relationships, you have to do a lot of work to accomplish things that should be really simple.

As for the URLs, yes they turn out to be damn ugly. But to be honest they are already quite ugly when you are using the where param.

Btw, I was unaware of /MyMainModel/1/MyModel?where={}&limit=30 so I tested it out. The problem with this is that it brings you only the data from MyModel and it omits MyMainModel or any other associated model. Still it's good to know.

Anyway, this is just my opinion based on the little experience I have. Don't mean to sound cocky or anything.

Thanks for the reply.

ybrodsky avatar May 10 '16 13:05 ybrodsky

Yes, you're right, it's actually a waterline limitation,

Even though where param can be quite ugly, you can pass conditions as follows /city?name=Buenos Aires, which is less verbose, but it also increases the complexity to try anything on the ?populate= param, which for me is highly unlikely to happen, i've trying to think something like, /region?populate[country?populate[cities?where={'name':'Buenos Aires'}]], that is quite unusual, not only the hard implementation itself, but for any Web API Design principles, which sails blueprint was meant to work as restful APIs.

About the /MyMainModel/1/MyModel?where={}&limit=30, yes, it omits MyMainModel, but as the expected, because you are requesting the child, unless that you request directly from the top, /MyMainModel/1, to catch all relations (but not deeply), furthermore, when you request the child relation directly like /MyModel you can get access to the parent as well, let me use a more practice example:

GET /user/2 =>

{
  id: 2,  
  name: 'user',
  pets: [{
    id: 1,
    name: 'Max'
  }]
}

GET /user/2/pets =>

[{
  id: 1,
  name: 'Max',
  userId: 2
}
...
]

GET /pet =>

[{
  id: 1,
  name: 'Max',
  userId: 2,
  owner: {
    id: 2,
    name: 'user'
  },
  otherRelations: [{}]
...
]

You just need to setup the belongsTo relation alias properly

Pet.belongsTo(User, {
  as: 'owner',
  foreignKey: {
    name: 'userId',
    as: 'owner'
  }
});

In your City => Country => Region example, you could do as follows: GET /countries =>

[{
  countryId: 4,
  name: 'Argentina',
  owner: {
    regionId: 5,
    name: 'South America'
  },
  cities: [{
    id: 1,
    name: 'Buenos Aires',
    countryId: 4,
  }]
}
...
]

It's actually more convenient instead of getting multiple deep levels, which is not actually a good practice more any restful api design.

Let me know if you got it, or i can do something about it.

cesardeazevedo avatar May 10 '16 22:05 cesardeazevedo

A country may have hundreds of cities and I will hardly ever need them all. What I might need is to bring a country and every city within with certain caracteristics. Again, we are talking about a filter in the contained model (city in this case).

I don't know if it's unusual to have such complex and twisted queries; I work at a travel agency and this kind of things come up all the time unfortunately.

Anyway, thanks for the replies, certainly got me to re-think some things that I took for granted.

ybrodsky avatar May 11 '16 18:05 ybrodsky