Using Scopes in Ability results in hundreds of queries #511

thebravoman · 2018-06-14T19:21:00Z

Issue is that when there is an error in the controller or the view then hundreds of queries are output to the console increasing the time waiting when an error occurs to about a minute which is unproductive.

Complete repo for reproducing is available at:
https://github.com/thebravoman/cancan_error

WIth the repo

bundle install
rails s

go to /course_sections/1/edit

The following error occurs

ActionView::Template::Error (undefined local variable or method `tt' for #<#<Class:0x00000000cf6b10>:0x00000000edef68>
Did you mean?  t):
    1: <% tt.tt %>

That is not the problem. The problem is that when an error occurs this is output in the console

Started GET "/course_sections/1/edit" for 127.0.0.1 at 2018-06-14 21:58:29 +0300
Processing by CourseSectionsController#edit as HTML
  Parameters: {"id"=>"1"}
  CourseSection Load (0.2ms)  SELECT  "course_sections".* FROM "course_sections" WHERE "course_sections"."id" = ? LIMIT ?  [["id", 1], ["LIMIT", 1]]
  Rendering course_sections/edit.html.erb
  Episode Load (0.1ms)  SELECT  "episodes".* FROM "episodes" LIMIT ?  [["LIMIT", 11]]
  Material Load (0.1ms)  SELECT  "materials".* FROM "materials" LIMIT ?  [["LIMIT", 11]]
  CACHE Episode Load (0.0ms)  SELECT  "episodes".* FROM "episodes" LIMIT ?  [["LIMIT", 11]]
  CACHE Material Load (0.0ms)  SELECT  "materials".* FROM "materials" LIMIT ?  [["LIMIT", 11]]
  CACHE Episode Load (0.0ms)  SELECT  "episodes".* FROM "episodes" LIMIT ?  [["LIMIT", 11]]
  CACHE Material Load (0.0ms)  SELECT  "materials".* FROM "materials" LIMIT ?  [["LIMIT", 11]]
  CACHE Episode Load (0.0ms)  SELECT  "episodes".* FROM "episodes" LIMIT ?  [["LIMIT", 11]]
  CACHE Material Load (0.0ms)  SELECT  "materials".* FROM "materials" LIMIT ?  [["LIMIT", 11]]
  CACHE Episode Load (0.0ms)  SELECT  "episodes".* FROM "episodes" LIMIT ?  [["LIMIT", 11]]
  CACHE Material Load (0.0ms)  SELECT  "materials".* FROM "materials" LIMIT ?  [["LIMIT", 11]]
  CACHE Episode Load (0.0ms)  SELECT  "episodes".* FROM "episodes" LIMIT ?  [["LIMIT", 11]]
  CACHE Material Load (0.0ms)  SELECT  "materials".* FROM "materials" LIMIT ?  [["LIMIT", 11]]
  CACHE Episode Load (0.0ms)  SELECT  "episodes".* FROM "episodes" LIMIT ?  [["LIMIT", 11]]
  CACHE Material Load (0.0ms)  SELECT  "materials".* FROM "materials" LIMIT ?  [["LIMIT", 11]]
  CACHE Episode Load (0.0ms)  SELECT  "episodes".* FROM "episodes" LIMIT ?  [["LIMIT", 11]]
  CACHE Material Load (0.0ms)  SELECT  "materials".* FROM "materials" LIMIT ?  [["LIMIT", 11]]
  Rendered course_sections/edit.html.erb (247.5ms)
Completed 500 Internal Server Error in 275ms (ActiveRecord: 2.8ms)

Here you can see a log of CACHE errors.

This project is a very simple example, but in a real life scenario, especially when the episodes and materials have some logic and globalize installed then hundreds of queries are output to the console and it takes about a minute.

This means that if there is and error on the server durring development we must wait about a minute which is unproductive.

The issue occurs probably because of

    can :access, Episode, Episode.all do |model_object|
    end
    can :access, Material, Material.all do |model_object|
    end

If this lines are changed with

    can :access, Episode do |model_object|
    end
    can :access, Material do |model_object|
    end

The issue will no longer occur

The text was updated successfully, but these errors were encountered:

duffyjp · 2018-06-20T18:19:09Z

https://github.com/CanCanCommunity/cancancan/wiki/Defining-Abilities

You just want

can :read, Episode
can :read, Material

The can :manage, :all you have later you probably want to get rid off too.

coorasse · 2018-06-22T08:58:27Z

I don't see what should be done here. The issue seems solved with @duffyjp suggestion

thebravoman · 2018-06-25T11:24:18Z

@coorasse @duffyjp I've used the Episode.all for an example of a scope but this happens with all scopes and that is the problem. The real example from the real project is with:

Episode.published

Which returned the episodes that are published.

So removing the scope is not a solution. There could be another scope. If a scope is used than the problem with thousands of queries occurs. If on the other hand a condition is used

['published_at < ?, Time.now]

than the queries are not printed.

But moving a scope to a condition is very hard, and sometimes impossible, because the scope could involve a more complex logic

duffyjp · 2018-06-25T14:53:46Z

@thebravoman I have several large projects that use cancancan extensively. I've made a rule that I only use condition hashes, and forbid blocks. As you get more and more rules, the extra effort is worth it. Ideally, if you have a published_at timestamp you also have a published boolean.

can :read, Episode, published: true

If not, use two rules.

can :read, Episode
cannot :read, Episode, published_at: nil

thebravoman · 2018-06-25T14:57:03Z

@duffyjp,
I completely agree that is a good rule of thumb. To have condition hashes and forbid blocks. We could rework the abilities to support this.

But the issue here is that by using blocks an undesired behavior occurs. Should we note that scopes should not be used in cancancan because of issues or it should be able to use scopes.

coorasse · 2018-07-06T04:49:03Z

Can you please provide a gist to reproduce the error @thebravoman ? You can find it in the pull request template. thanks

thebravoman · 2018-07-06T15:41:23Z

I'v provided a whole project for reproducing it. gist is not enough.

…

On Fri, Jul 6, 2018, 07:49 Alessandro Rodi ***@***.***> wrote: Can you please provide a gist to reproduce the error @thebravoman <https://github.com/thebravoman> ? You can find it in the pull request template. thanks — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#511 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQDBX4P06AWun9iLEzCYYzSHlmLU2yxks5uDuxBgaJpZM4UohMx> .

sarink · 2018-12-05T09:20:37Z

This is happening to me too

# tag.rb
class Tag < ApplicationRecord
  scope :preset?, -> { where(scenario: nil) }
end

# ability.rb
def initialize
  can :read, Tag, id: Tag.preset?.pluck(:id)
end

and then literally hundreds of SELECT 1 AS one from "Tags" ...

thebravoman · 2018-12-05T11:56:01Z

Here is what we've done since then. We've separated the large ability on many small abilities that we are merging and we are trying to remove scopes from the Abilities. There are still some but at least we are trying.

…

On Wed, 5 Dec 2018 at 11:20, Kabir Sarin ***@***.***> wrote: This is happening to me too # tag.rb class Tag < ApplicationRecord scope :preset?, -> { where(scenario: nil) } end # ability.rb def initialize can :read, Tag, id: Tag.preset?.pluck(:id) end and then literally *hundreds* of SELECT 1 AS one from "Tags" ... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#511 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQDBVwhK4XxVlGBhXCSf7g9GZKyw7Bxks5u14_ogaJpZM4UohMx> .

-- +359 878 93 30 94 Училище по роботика "Robopartans" www.robopartans.com

sarink · 2018-12-05T17:39:55Z

I'm not sure how that would help my situation? I need to only allow tags which do not belong to a scenario. If I can't use a block (because it breaks accessible_by), and I can't use a scope (because it's tragically slow), how am I supposed to do this?

thebravoman · 2018-12-05T18:24:25Z

1. Don't use cancancan - it is difficult for me to migrate and I kind of like the qay cancancan could be used, but I know of at least one project that has stopped using cancancan 2. Migrate the scope to an sql that could be uswd in the ability. if the are not many joins basically it is doable.

…

On Wed, Dec 5, 2018, 19:40 Kabir Sarin ***@***.*** wrote: I'm not sure how that would help my situation? I need to only allow tags which do not belong to a scenario to be read. If I can't use a block, and I can't use a scope, how am I supposed to do this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#511 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQDBYwhgKuRqWWsneNTBm1hsBEpN940ks5u2ATxgaJpZM4UohMx> .

sarink · 2018-12-05T23:35:37Z

Is there some reason this has to happen? Why would using a scope be fundamentally different than writing the raw SQL? Can't you literally call to_sql on a scope? If option #2 is true, then this seems like a bug.

thebravoman · 2018-12-06T19:42:53Z

Yes. I think it is actually a bug.

I can not understand how the scope causes these errors to occur, but if you use an sql statement no errors occur. These are not errors actually - these are 2 minutes long dumps in the log that completely block your server and make you development a nightmare and this happens only if there is a real error - like 500.

coorasse · 2019-02-21T16:49:45Z

The problem is related to the fact that the scopes are evaluated every time the ability file is used (i.e. a call to can? or accessible_by). In this case by load_and_authorize_resource. SQL statements, instead, are not. I never used scopes, because they seem just unnecessary since you can use raw SQL queries but to me, this is definitely a bug. They should not be evaluated immediately.

coorasse · 2019-02-21T17:10:03Z

Looks like this thread is related: rails/rails#30497

coorasse · 2019-02-21T18:37:02Z

the fix will be released in version 3.0.0

thebravoman · 2019-02-25T12:15:29Z

Great. Thanks @coorasse. Is 3.0.0 planned for the near future?

coorasse · 2019-02-25T13:09:41Z

yes. in the meantime you can try the branch feature/3.0.0. this would help me also to identify other issues before the 3.0.0 release. A release candidate will be out very soon.

robinboening · 2022-06-14T14:13:12Z

@thebravoman I have several large projects that use cancancan extensively. I've made a rule that I only use condition hashes, and forbid blocks. As you get more and more rules, the extra effort is worth it. Ideally, if you have a published_at timestamp you also have a published boolean.
can :read, Episode, published: true
If not, use two rules.
can :read, Episode
cannot :read, Episode, published_at: nil

I just ran into an issue, unrelated to the original issue raised here, where I had to use use two rules even though I was using a boolean attribute like in your first example.

For me this was needed in order to allow accessing Page objects, but not restricted ones.

can :read, Page
cannot :read, Page, restricted: true

coorasse self-assigned this Feb 21, 2019

coorasse added the bug label Feb 21, 2019

coorasse mentioned this issue Feb 21, 2019

Avoid multiple queries on session dump #566

Merged

coorasse added the next major release label Feb 21, 2019

coorasse closed this as completed Feb 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Using Scopes in Ability results in hundreds of queries #511

Using Scopes in Ability results in hundreds of queries #511

thebravoman commented Jun 14, 2018

duffyjp commented Jun 20, 2018

coorasse commented Jun 22, 2018

thebravoman commented Jun 25, 2018

duffyjp commented Jun 25, 2018

thebravoman commented Jun 25, 2018

coorasse commented Jul 6, 2018

thebravoman commented Jul 6, 2018 via email

sarink commented Dec 5, 2018

thebravoman commented Dec 5, 2018 via email

sarink commented Dec 5, 2018 •

edited

Loading

thebravoman commented Dec 5, 2018 via email

sarink commented Dec 5, 2018 •

edited

Loading

thebravoman commented Dec 6, 2018 •

edited

Loading

coorasse commented Feb 21, 2019 •

edited

Loading

coorasse commented Feb 21, 2019

coorasse commented Feb 21, 2019

thebravoman commented Feb 25, 2019

coorasse commented Feb 25, 2019

robinboening commented Jun 14, 2022

Using Scopes in Ability results in hundreds of queries #511

Using Scopes in Ability results in hundreds of queries #511

Comments

thebravoman commented Jun 14, 2018

duffyjp commented Jun 20, 2018

coorasse commented Jun 22, 2018

thebravoman commented Jun 25, 2018

duffyjp commented Jun 25, 2018

thebravoman commented Jun 25, 2018

coorasse commented Jul 6, 2018

thebravoman commented Jul 6, 2018 via email

sarink commented Dec 5, 2018

thebravoman commented Dec 5, 2018 via email

sarink commented Dec 5, 2018 • edited Loading

thebravoman commented Dec 5, 2018 via email

sarink commented Dec 5, 2018 • edited Loading

thebravoman commented Dec 6, 2018 • edited Loading

coorasse commented Feb 21, 2019 • edited Loading

coorasse commented Feb 21, 2019

coorasse commented Feb 21, 2019

thebravoman commented Feb 25, 2019

coorasse commented Feb 25, 2019

robinboening commented Jun 14, 2022

sarink commented Dec 5, 2018 •

edited

Loading

sarink commented Dec 5, 2018 •

edited

Loading

thebravoman commented Dec 6, 2018 •

edited

Loading

coorasse commented Feb 21, 2019 •

edited

Loading