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

Fetch associations for an array of models when autoFetch is off (similar to Mongoose's populate) ? #120

Closed
cheapsteak opened this issue Apr 15, 2013 · 5 comments

Comments

@cheapsteak
Copy link

I have two models with a many to many relationship, routes and stops (for buses)

given an api like below:

/routes   
/routes/12,13,22,555,103,23,19/stops  
/stops   
/stops/23,32/routes   

I don't want /routes nor /stops to autoload, there's too much data, but I do want /routes/:ids/stops to autoload, and same for /stops/:ids/routes.

Is this possible?

Also, what's a way to do it without using autoload?
I've gotten this far:

app.get( '/api/routes/:ids/stops', function (request, response){
    var ids = request.params.ids.split(',');
    return request.models.route
        .find({id: ids})
        .each(function (route) {
            route.getStops(function(err, stops){ 
                route.stops = stops; // <--- need to make sure this gets called before returning
            });
        })
        .get(function(routes) {
            return response.send( routes );
        });
});

but I'm currently stuck trying to figure out how to make sure all the route.getStops have returned before sending the response.

Would I need a promise library to deal with this?


edit: dug around the source, found this but I don't seem to be using it correctly

request.models.route.settings.set('instance.autoFetch', true);
request.models.stop.settings.set('instance.autoFetch', true);

setting this doesn't seem to affect anything


edit: setting it as an option passed to find doesn't seem to work either

return request.models.route.find({id: ids}, {autoFetch: true}, function(err, routes){
    return response.send( routes );
});

edit: ~~~possibly solved?~~~ (nvm, apparently this messes up caching)
changed line 245 of Model.js from

autoFetch      : (options.autoFetchLimit === 0 ? false : opts.autoFetch)

to

autoFetch      : (options.autoFetchLimit === 0 ? false : options.autoFetch)

to allow specifying autoFetch as an option when calling find.

was this a typo or was the autoFetch not intended to be override-able?


edit: apparently changing that messes up caching.

request.models.route.find({id: ids}, {autoFetch: false})

or

request.models.route.find({id: ids}, {autoFetch: true})

whichever one gets called first ends up determining autoFetch forever.

setting instance settings via request.models.model.settings.set didn't work either

app.get( '/api/routes/:ids', function (request, response){
    request.models.route.settings.set('instance.autoFetch', false); 
    request.models.route.find({id: ids}, {autoFetch: false})
}

app.get( '/api/routes/:ids/stops', function (request, response){
    request.models.route.settings.set('instance.autoFetch', true);
    request.models.route.find({id: ids}, {autoFetch: true})
});

The setting does change but it doesn't seem to affect the output

@dresende
Copy link
Owner

I'm sorry for not replying earlier but I'm a little short in time. I have to take some time to test more properly before answering you.

@dresende
Copy link
Owner

It seems there is some trouble with passing autoFetch to Model.find. I'm going to make some tests and fix this.

@dresende
Copy link
Owner

This fix doesn't magically solve your problem. I would advise you to do this way:

  1. Don't enable autoFetch on Model definition, keep cache enabled (default);
  2. When just wanting to fetch a Route info or Stop info, just go on and do Model.find(). It shouldn't fetch the association items and that's what you want;
  3. When you need to fetch a Route and related Stops (or vice-versa), use Model.find(..., { cache: false, autoFetch: true }).

@cheapsteak
Copy link
Author

Yup, that works great, thanks!
How big of a deal is caching? Would it be a good idea to try to implement separate instances depending on whether 'autoFetch' is on, or is that going to open up a can of worms?

@dresende
Copy link
Owner

Yeah, that might open the can, I have to think a bit more about the cache keys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants