7

I have a large ability file that decides what exactly users can do by searching from a table of 'Roles'. Each role corresponds to something a particular user can do, for example, being able to add a project or being able to edit the main company record.

At the moment, every controller action that runs load_and_authorize_resource goes through >30 statements like:

ability.rb (>30 times)
Role.where("user_id = ? AND role = ? AND roleable_type = ? AND roleable_id IS NULL", user.id, "delete", "task").last.present? ? (can :destroy, Task) : nil

This is a horribly inefficient solution because the server is running >30 queries before it even does anything.

The best way to do this would only be to run the queries that need running based on what the controller and view require. Is there a way to do this?

ronalchn
  • 12,225
  • 10
  • 51
  • 61
sscirrus
  • 55,407
  • 41
  • 135
  • 228
  • Uh oh, I'm going to have to audit my own cancan usage. Out of curiosity, does it do it in both dev and production environments? – miked Feb 04 '12 at 22:23
  • It's hard for me to grok what all is going on in your associations with that one line. But perhaps you could eager load all of a user's roles with something like `User.includes(:roles).find(@user.id)`. Then one SQL query will hold the user's roles in memory for you to process. – danneu Feb 05 '12 at 03:22
  • @danneu - I'll try your suggestion and see if it reduces the number of queries. Basically, each 'role' gives a user a specific permission - this allows user abilities to be infinitely customizable (but it does, as you can see, result in some fiddly performance situations). – sscirrus Feb 05 '12 at 07:39
  • 1
    Seconding @danneu's suggestion – rickypai Aug 03 '12 at 20:30

1 Answers1

4

It's partly the way you wrote the role tests. Instead of writing > 30 times:

Role.where("user_id = ? AND role = ? AND roleable_type = ? AND roleable_id IS NULL", user.id, "delete", "task").last.present? ? (can :destroy, Task) : nil

You can instead query for all of a users roles at once:

@user_roles = user.roles.all

Then individually test for each role:

can :destroy, Task if @user_roles.detect {|u| u["role"] == "delete" && u["roleable_type"]=="task" }

Because all roles are read into memory in a single query, you don't have 30 queries.

ronalchn
  • 12,225
  • 10
  • 51
  • 61