-
Notifications
You must be signed in to change notification settings - Fork 91
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
Optimize lookups of relationships where ID is sufficient #30
Comments
Could this not be done with |
jsonapi-serializers is not coupled at all to ActiveRecord at the moment, so no, not if we wanted to maintain that. We could potentially use those methods and have a fallback for the more generic case. |
Ah, I see. I suppose we could use the adapter pattern to solve the problem for users while maintaining jsonapi-serializers' independence. e.g. jsonapi-serializers-activerecord could register itself and override whatever smart defaults are provided for the generic case. |
Yup that could work. I also like the more lightweight solution of just duck-type checking if the methods exist and then falling back, since there's not a lot of other coupling we need to do so |
This problem is particularly gruesome with e.g. Sequel, which eagerly typecasts entire records (as opposed to ActiveRecord's lazy typecasting). It gets pretty ugly when you need to include types and IDs for a tall and wide has_many collection. I've been looking at JSONAPI::Serializer#has_one_relationship and JSONAPI::Serializer#has_many_relationship. It seems to me that there should be a hint passed to those methods when you only need an object "skeleton." Instead of JSONAPI::Serializer#evaluate_attr_or_block doing this: object.send(attr_or_block) I really want it to look for a hook in my (base) serializer that I've overridden to do this, for has_many skeletons: def has_many_skeletons(attr)
reflection = object.model.association_reflection(attr)
klass = reflection.associated_class
reflection.association_dataset_for(object).select(klass.primary_key).all
end Or this, for has_one skeletons: def has_one_skeleton(attr)
reflection = object.model.association_reflection(attr)
klass = reflection.associated_class
reflection.association_dataset_for(object).select(klass.primary_key).first
end You might be able to get crafty for has_one and avoid a query entirely for the simple cases: def has_one_skeleton(attr)
reflection = object.model.association_reflection(attr)
klass, id = reflection.associated_class, object.send(reflection.default_key)
klass.(klass.primary_key_hash(id))
end I'll admit to getting a little lost in Sequel's association reflection documentation, so there might be better ways to accomplish the above. |
Consolidating issues:
We need a mechanism to reference
post.author_id
rather thanauthor.id
when only finding the ID for relationshipdata
andlinks
. Right now, we naively always reference the.id
method which, in the ActiveRecord world, forces a load of that record (unless it has been preloaded via.includes(:author)
when loading the parent).A few options:
<relationship name>_id
method and fallback to the object'sid
method.id_lookup: :author_id
. This idea could be part of the above option.The text was updated successfully, but these errors were encountered: