ryanb / cancan

Authorization Gem for Ruby on Rails.
MIT License
6.26k stars 783 forks source link

Optimisation: when providing a scope as conditions, unnecessary DB hits are issued (cc2.0) #1000

Open gamov opened 10 years ago

gamov commented 10 years ago

The two following lines issues unnecessary DB hits (calling empty? and present? on a AR:Relation): https://github.com/ryanb/cancan/blob/2.0/lib/cancan/rule.rb#L42 changed to

@conditions.is_a?(ActiveRecord::Relation) || @conditions.empty? ? true : @base_behavior

maybe just testing if it's a hash and empty is enough… then https://github.com/ryanb/cancan/blob/2.0/lib/cancan/rule.rb#L59 changed to

@conditions.is_a?(ActiveRecord::Relation) || @conditions.present?

https://github.com/ryanb/cancan/blob/2.0/lib/cancan/rule.rb#L51 changed to

@block.nil? && conditions? && !@conditions.kind_of?(Hash) && !@conditions.is_a?(ActiveRecord::Relation)

lastly https://github.com/ryanb/cancan/blob/2.0/lib/cancan/rule.rb#L63 to

@block || (!@conditions.is_a?(ActiveRecord::Relation) && conditions?)

I'm not making a pull request since I'm not sure of the implications of those changes.

PS: I'm posting this here since it's related to cc2.0

xhoy commented 10 years ago

Dear submitter, Since cancan/raynB hasn't been active for more than 6 months and no body else then ryam himself has commit permissions the cancan project is on a stand still. Since cancan has several issues including missing support for rails 4 cancan is moving forward to cancancan. More details on: #994

If your feel that your pull request or bug is still applicable (and hasn't been merged in to cancan) it would be really appreciated if you would resubmit it to cancancan (https://github.com/cancancommunity/cancancan)

We hope to see you on the other side!