Using .where on associations performs API call twice. Once without .where params, and once with .where params
tolgap opened this issue · 5 comments
Given the following construction:
# app/models/user.rb
class User < ActiveResource::Base
has_many :investments
end
# app/models/investment.rb
class Investment < ActiveResource::Base
schema do
string :status
end
belongs_to :user
end
user = User.find(1)
user.investments.where(status: "success")
running this on my server causes three API calls:
# The User.find call:
- -> /user/1.json
# user.investments line
- -> /investments.json?user_id=1
# .where(status: "success") line
- -> /investments.json?status=success&user_id=1
I would have expected only 2 api calls. One for User.find
and one for the entire user.investments.where
. If you take a look at what method is called for user.investments
, we notice that find_every
is called on an association collection. But this collection is immediately ran against the API:
activeresource/lib/active_resource/base.rb
Line 1075 in 71f349a
There is no chance to apply the
where
params on the collection before performing it, as far as I understand it?This doesn't change what you found, but there is a workaround that might let you have only 2 api calls:
user = User.find(1)
investments = Investment.where(status: "success", user_id: user.id)
This obviously isn't ideal since it'd be nice to just leverage associations.
You could simulate those associations (sort of) by doing something like:
class User < ActiveResource::Base
def investments(clauses = {})
Investment.where({user_id: id}.merge(clauses))
end
end
user = User.find(1) # makes the first API call
user.investments(status: "success") # makes the second
Sorry if I'm stating the obvious - maybe it'll help others at least.
Looking at the code, the real problem is that there are two where
methods.
There's a where
class method provided by ActiveResource::Base
. It's what's being used when you do Investment.where(status: "success")
.
The other where
is an instance method available to instances of ActiveResource::Collection
.
That method calls ActiveResource::Base.where
, as seen here:
def where(clauses = {})
raise ArgumentError, "expected a clauses Hash, got #{clauses.inspect}" unless clauses.is_a? Hash
new_clauses = original_params.merge(clauses)
resource_class.where(new_clauses)
end
When you do...
user = User.find(1)
user.investments.where(status: "success")
..the where
can only work on an instance of a Collection
. Said another way, it's simply too late to avoid the API call that populated that Collection
instance.
To make it avoid that extra API call a few things would need to happen:
Before instantiating a new Collection
(ie, the investments
in user.investments
, you'd need to determine if, once called, where
was also going to be called. If so, you'd need then merge those where
options into the API call for investments
.
It's not impossible, but it's not trivial either. You'd need sometime like AREL to have it behave properly for most cases. Maybe I'm wrong?
@elbowdonkey You are correct that your workaround works. It's basically what I'm doing right now.
It would be nice to have something like ActiveRecord::Associations::CollectionProxy that would delegate performing the actual API call, until the result is required (like doing any Enumerable
method).
I agree, that'd be pretty useful. I think, unfortunately, that ActiveResource just hasn't kept up the same pace of innovation over the years as ActiveRecord. I still use ActiveResource, and still think it's useful, but it could use some modernization.
I also didn't even consider CollectionProxy
maybe something similar could be adapted to work for ActiveResource as well?
This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If it is an issue and you can still reproduce this error on the master
branch,
please reply with all of the information you have about it in order to keep the issue open.
If it is a pull request and you are still interested on having it merged please make sure it can be merged clearly.
Thank you for all your contributions.