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

Optimize lookups of relationships where ID is sufficient #30

Open
fotinakis opened this issue Oct 13, 2015 · 5 comments
Open

Optimize lookups of relationships where ID is sufficient #30

fotinakis opened this issue Oct 13, 2015 · 5 comments

Comments

@fotinakis
Copy link
Owner

Consolidating issues:

We need a mechanism to reference post.author_id rather than author.id when only finding the ID for relationship data and links. 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:

  • Automatically try the relationship's <relationship name>_id method and fallback to the object's id method.
  • Provide a way to hint at the relationship method name, like id_lookup: :author_id. This idea could be part of the above option.
@garrettlancaster
Copy link
Contributor

Could this not be done with Post.reflect_on_association(:author).association_foreign_key?

@fotinakis
Copy link
Owner Author

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.

@garrettlancaster
Copy link
Contributor

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.

@fotinakis
Copy link
Owner Author

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 jsonapi-serializers-activerecord might be too heavyweight right now.

@mwpastore
Copy link
Contributor

mwpastore commented Nov 14, 2018

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.

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

No branches or pull requests

3 participants