sideloaded associations don't have their collection ids
wayne-o opened this issue · 18 comments
if you see here:
http://api.test.sonatribe.com/listingevents
you can see that artists are side loaded. cool. however, the artists don't have the images property.
If you drill down to one of the artists you can see it here:
http://api.test.sonatribe.com/artists/54e34a2de4dbefd62c409ad2
While I don't want the associations loaded that deep, when ember receives the sideloaded artist it thinks it has it all so it doesn't try and fetch the images when I drill into the artist.images property. I would have thought that would be lazily loaded but it doesn't.
I could be wrong with all this, in which case please accept my apologies. I have asked this question on SO if there is any detail I may have left out please let me know:
Unless I am supposed to do something like this? https://github.com/lfridael/ember-student-example/blob/master/app/routes/student.js
just seems a bit dirty?
Hi @wayne-o the solution is, that you would actually provide a links object in the payload, which describes the deeply nested images in the form of a url to Ember. With async:true enabled on the relationship in the Ember model and a url from where to fetch the records provided in the links property of the record, Ember knows to load the records (more or less) lazily.
(See http://jsonapi.org/format/ for a general idea on how to use links - the specific implementation is a bit different in Ember if I remember correctly...).
Does that help you?
Ah, and btw... the IDs are not in the rest response, because in order to fetch the IDs Waterline would actually fetch all the records (at least until https://github.com/balderdashy/waterline/issues/532 get's resolved) and that would give you a pretty big performance impact.
Hi,
Thanks for the response - so how do I get sails to respond with the links in the JSON? Is there a param I can send to Sails that will give me the links or is this a manual thing?
Great piece of work btw :)
In the Ember service included in this repo, there's a function "linkAssociations" that should do the trick. I just needs to get called in the right moment and you'll probably need to modify the blueprints to do just that. If you figure it out and this looks like a general bug in the blueprints, please also consider making a pull request.
On it like a car bonnet. I should be able to test today - if it is a bug I won't be able to push a pull until next week - hope that's ok :)
OK so this works for URLs like:
http://api.test.sonatribe.com/artists/54e387bbb6cc4b42308e10f2/listingevents
Would it be bad practice to have it specify links to 3rd level associations by default?
So when going here:
http://api.test.sonatribe.com/listingevents
we would get:
top level: listing events
2nd level: artists (links to associated models (images)), locations (links to associated models)
Or would that all be too taxing on the server?
Ah just seen your reply, i'll take a look :)
@wayne-o It would actually be good practice to have the links on the 3rd level by default. Creating the links property is easy work for the server, compared to indexing or sideloading the records.
Referencing this to include it in the discussion, as it's related: https://github.com/mphasize/sails-ember-blueprints/issues/17
It would actually be good practice to have the links on the 3rd level by default. Creating the links property is easy work for the server, compared to indexing or sideloading the records.
@mphasize I didn't fully understand the links-based solution, would the response look something like:
GET /api/v1/users/51
{
user: {
id: 51,
name: 'Alex',
posts: [1] //this one is currently included
},
posts: [{
id: 1
title: 'A trouble maker',
//comments: [10, 11, 12]// instead of this we will have links
links: {
comments: '/api/v1/comments?ids[]=10&ids[]=11&ids[]=12
}
}],
comments:[...]//this currently isn't included and shouldn't be anyway
}
Am I missing something?
@anasyb almost, the response would actually look like this:
GET /api/v1/users/51
{
user: {
id: 51,
name: 'Alex',
posts: [1] //this one is currently included
},
posts: [{
id: 1
title: 'A trouble maker',
links: {
comments: '/api/v1/posts/1/comments'
}
}]
}Linked association/relation records are not explicitly defined, the client has to send another request to find out if there are any, how many and which. But in my previous experience, Ember Data will send the request only when you actually try to get a relationship property.
I guess that should do... very excited to test this out when its ready.
I am curious how would ember behave in case post 1 was directly loaded in a previous view, I guess it will replace the old record which had the comments array with one which only have the link.
@mphasize I found a very simple and efficient solution:
- partially fix waterline by fixing toObject and making it get its act together and stop omitting PK arrays of non-populated associations. I did this (temporary) by commenting the line to blame for all this mess. A flag can be set on the model to switch it on/off, like
isAssociationVisible. I really think this should be considered a bug in toObject... one would expect to association PK arrays to be accessible in the JSON. - fix the blueprints by avoiding a call to populateAll as following
var query = Model.findOne( pk );
//query = actionUtil.populateEach( query, req );
query.exec( function found( err, matchingRecord ) {- if sideload option is true, we now can load the associated records by a find for each 1st level association. Using the example json above:
//assuming asocModelIdent='post'
req._sails.models[ asocModelIdent ].find({id:userRecord.posts}).exec(function(err, records)
//set json.posts to records
)I think this is as expensive as asking waterline to populate the associations.
What do you think?
@anasyb Looks interesting. Are you sure that userRecord.posts contains the IDs when you don't populate the association? And yes, it probably wouldn't be more expensive.
(It should be, because a single query with a proper join should be faster, but the last time I took a deep dive into Waterline I saw a nasty query integrator thing – which they need for the cross-database integration of queries, but which is also awfully slow... comparatively speaking.)
Are you going to file an issue for your findings above in the Waterline repo?
@mphasize yes am sure it contain the IDs, at least it did when I tried it :)
I even over-rided the toJSON
The issue is with toObject... see this fresh from the console:
User.find().exec(function(err, users){
console.log(JSON.stringify(users[0]))
})
//{ id:1, name:'Susan'} //no post IDs!
User.find().exec(function(err, users){
console.log(users[0].posts);
})
//['1', '2', '3', add:[Function add], remove:[Function remove]] //but actually here they are!!
User.find().exec(function(err, users){
users[0].toJSON="I won't allow you to use sails toJSON, just use the native";
console.log(JSON.stringify(users[0]))
})
//{ id:1, name:'Susan', posts:[1,2,3]} //post IDs are back!- I am going to file an issue on waterline soon and link it to your issue. Basically they are very similar, only this one is about JSON purposefully omitting association arrays even when they are available (1st order associations), and the one you filed is about populate ignoring associations.
- my worry (though I didn't confirm it) with the links appraoch is that it might force ember to re-fetch records it already fetched in the past. This could happen because we will be sending links in place of IDs. If the associated records are something with images (like avatars next to comments)... this good go ugly expensive.
@anasyb iirc I read something that recent Ember Data versions should be smart enough not to re-fetch the associations, but I will be working with exactly that a bit more over the next weeks and be sure that I will fix more stuff when I find it. I'll take a look at your discovery and see what I can do with it.