Stuck on avoiding N+1 with associations

[Signum]

I'm so happy with CanCanCan. My Rails views are so easy to understand now. Thanks.

However on one page I have an ugly case of N+1 queries. And I fail to find a solution. Help would be very welcome.

The application runs https://screenshots.debian.net/ – a web site managing screenshots for Debian/Linux software packages that I have been maintaining for the community for ~10 years.

I basically have two models:

1. Packages (a piece of software)
2. Screenshots (multiple images for a package)

A Package has_many :screenshots.

Screenshots can be in different states. They can be new and need to be moderated (approved=false). They might be hidden temporarily (hidden=true). So depending on whether you are logged in and who you are, you may see different screenshots.

My ability file looks like:

# Everybody can see public screenshots
can :view, Screenshot, approved: true, hidden:false

if user.present? # Logged-in users
  # Allow to view own uploads (even not-yet-approved)
  can :view, Screenshot, user_id: user.id

  # Admins can see any screenshots
  if user.admin_role?
    can :view, Screenshot
  end
end

In the view you see at https://screenshots.debian.net/packages?show=with the logic is basically:

@packages = Package.where…
@packages.each do |pkg|
  = image_tag(pkg.screenshots.accessible_by(current_ability, :view).first)

The first line gets packages:

SELECT "packages".* FROM "packages" WHERE…

The third line however is the culprit that does N+1 queries like:

SELECT "screenshots".* FROM "screenshots"
WHERE "screenshots"."package_id" = $1 AND "screenshots"."approved" = $2 AND "screenshots"."hidden" = $3
LIMIT $4  [["package_id", 150800], ["approved", true], ["hidden", false], ["LIMIT", 1]]

So the obvious question is: how would I properly use .accessible_by while avoiding N+1?
I tried various ways of includes(:screenshots) and joining screenshots but none of my approaches were even valid for ActiveRecord.

Thank you very much in advance for any input. I've been stuck on this for days.

…Christoph

[coorasse]

Your issue is not related to cancancan specifically. You want to eager load an association with scope. I wrote about this some time ago: https://medium.com/@coorasse/eager-load-associations-with-parameters-in-rails-6e9fd7b65923

I expect something like:

packages = Package.where(...)
ActiveRecord::Associations::Preloader.new.preload(packages, : screenshots, Screenshot.accessible_by(...))

[Signum]

Thanks @coorasse. You are right, that's not a specific CanCanCan issue. I just wondered if there is a common programming pattern for using accessible_by() in this context.

Your idea with preload works half way. My code:

@packages = Package.where(…
@packages = @packages.paginate(…
ActiveRecord::Associations::Preloader.new.preload(@packages.all.to_a, :screenshots, Screenshot.accessible_by(current_ability, :view))

That leads to the SQL I would expect:

SELECT COUNT(*) FROM "packages" WHERE …
SELECT "screenshots".* 
  FROM "screenshots"
  WHERE "screenshots"."approved" = $1 AND "screenshots"."hidden" = $2
  AND "screenshots"."package_id" IN ($3, $4, $5, $6, $7, $8, $9, $10, $11) 
  [["approved", true], ["hidden", false], ["package_id", 44197], ["package_id", 153], ["package_id", 190],…]

Interestingly there are still N+1 queries going on for each Package in the view when I do…

= img_tag(pkg.screenshots.accessible_by(current_ability, :view).first)

That is the query that gets run N+1 times:

SELECT "screenshots".*
FROM "screenshots"
WHERE "screenshots"."package_id" = $1 AND "screenshots"."approved" = $2 AND "screenshots"."hidden" = $3
ORDER BY "screenshots"."id" DESC
LIMIT $4
[["package_id", 44197], ["approved", true], ["hidden", false], ["LIMIT", 1]]

So something in ActiveRecord thinks that it needs to query the database again instead of just taking the preloaded data.
Seems I need to learn more about ActiveRecord to understand this.

Thanks for the hint with preload anyway. Although I read that it's highly debated whether that "hidden feature" should be used. :-)

[coorasse]

When you write

pkg.screenshots.accessible_by(current_ability, :view).first

it will not use the preloaded association. after you preloaded, you can simply do:

pkg.screenshots.first

pkg.screenshots has already been preloaded with only the filtered screenshots. this should do. if .first runs again a query try with [0]

[Signum]

Thanks again for the hint. I thought that the preloader would just load the screenshots specified by accessible_by(). And that accessing say @package.first.screenshots would still refer to all associated screenshots.

But if I understand correctly the preloader changes the meaning of the Package.screenshots attribute. It no longer refers to all associated screenshots (as per the foreign key in the model). Intead it makes .screenshots refer to exactly the subset that was queried by the accessible_by() call.

Another mistake I made was writing…

ActiveRecord::Associations::Preloader.new.preload(
  @packages.all.to_a, :screenshots, Screenshot.accessible_by(current_ability, :view))

…instead of…

ActiveRecord::Associations::Preloader.new.preload(
  @packages, :screenshots, Screenshot.accessible_by(current_ability, :view))

That of course does not enrich the @packages object because it was turned into an array.
One line to rule them all. Thanks a lot!