Draper forces DB queries on decorated model's associations

[swiercz]

Assume:
User has_many posts
Post has_many comments

When fetching users from DB with preloading of posts and comments:

@users = User.where(....).includes(posts: [:comments]).decorate

The AR properly preloads users, posts and comments (3 queries) but decorated users' collection forces new DB queries when rendering view.

It seems that objects from decorated collection ignore the fact that associations are preloaded by AR and fore new DB query for each one. This results with a performance drop.

RABL:

child @users do
  extends 'users/_base'
end

User partial

attributes :email
child :posts do
  extends 'posts/_base'
end

Post partial

attributes :title
child :posts do
  extends 'comments/_base'
end

Comment partial

attributes :text, :created_at

User decorator

class UserDecorator < Draper::Decorator
  delegate_all
  def posts
    object.posts.decorate
  end
end

Post decorator

class PostDecorator < Draper::Decorator
  delegate_all
  def comments
    object.comments.decorate
  end
end

[dimroc]

👍

We're pretty aggressive with our eager loading so this is causing a pretty big hit to perf on certain views.

Decorating the individual elements rather than the collection avoids this issue.

[jtomaszewski]

Has this been fixed? I'm experiencing the same issue as @dimroc on draper 2.1.0.

I needed to use the same workaround, to avoid the problem:

Decorating the individual elements rather than the collection avoids this issue.

[darrinholst]

I just noticed this as well. @jcasimir sorry for the 2 year old question, but do you remember why this was closed?

[seanlinsley]

Given that there isn't a commit or PR linked to this ticket, I suspect it's still an issue.

[codebycliff]

I was able to reproduce this behavior and I fixed it by using decorates_associated instead of overriding the method:

class UserDecorator < Draper::Decorator
  delegate_all
  decorates_associated :posts
end

class PostDecorator < Draper::Decorator
  delegate_all
  decorates_associated :comments
end

Were the rest of you experiencing this issue also overriding the association method in the decorator? If so, you should be able to change it to the code above. If there is another scenario that is causing this, please let me. If you could provide an example app, that would be even better.

[dimroc]

I tried this in two projects with version draper 3.0.0.pre1. In the first application (straight Rails and ActiveRecord) I got eager loading to work even without the `decorates_associated`. Everything was great there, and I don't remember that being the case. In my second application I had mixed results. I also got eager loading working even without the `decorates_associated` but I also had it fail with it. This code base has many requests intertwined with ElasticSearch, and eager loading across two data stores doesn't make for a clean example. Perhaps someone else can come up with a failing scenario. For me, it looks like it's been resolved.

…

On Wed, Apr 5, 2017 at 11:11 PM Cliff Braton ***@***.***> wrote: I was able to reproduce this behavior and I fixed it by using decorates_associated instead of overriding the method: class UserDecorator < Draper::Decorator delegate_all decorates_associated :postsend class PostDecorator < Draper::Decorator delegate_all decorates_associated :commentsend Were the rest of you experiencing this issue also overriding the association method in the decorator? If so, you should be able to change it to the code above. If there is another scenario that is causing this, please let me. If you could provide an example app, that would be even better. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#646 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAmw8bXwdugS5pHX73BA_LdgUEOu5Bfoks5rtFfYgaJpZM4DKmCq> .

[darrinholst]

I couldn't find the part of our app that I was seeing this in now. It appeared to be working where I thought the problem was now. It was a low traffic part so I guess I didn't bother to document it anywhere. I'd call it resolved for now. I'll make sure to get more details if/when I see it again.

[codebycliff]

Okay, I'm going to close this then. Thanks for the responses.