diff --git a/lib/cancan.rb b/lib/cancan.rb index 825b336..a6bdce6 100644 --- a/lib/cancan.rb +++ b/lib/cancan.rb @@ -5,3 +5,4 @@ require 'cancan/resource_authorization' require 'cancan/controller_additions' require 'cancan/active_record_additions' require 'cancan/exceptions' +require 'cancan/query' diff --git a/lib/cancan/ability.rb b/lib/cancan/ability.rb index c376622..8b2c1b7 100644 --- a/lib/cancan/ability.rb +++ b/lib/cancan/ability.rb @@ -182,86 +182,9 @@ module CanCan @aliased_actions = {} end - # Returns an array of arrays composing from desired action and hash of conditions which match the given ability. - # This is useful if you need to generate a database query based on the current ability. - # - # can :read, Article, :visible => true - # conditions :read, Article # returns [ [ true, { :visible => true } ] ] - # - # can :read, Article, :visible => true - # cannot :read, Article, :blocked => true - # conditions :read, Article # returns [ [ false, { :blocked => true } ], [ true, { :visible => true } ] ] - # - # Normally you will not call this method directly, but instead go through ActiveRecordAdditions#accessible_by method. - # - # If the ability is not defined then false is returned so be sure to take that into consideration. - # If the ability is defined using a block then this will raise an exception since a hash of conditions cannot be - # determined from that. - def conditions(action, subject, options = {}) - relevant = relevant_can_definitions(action, subject) - unless relevant.empty? - if relevant.any?{|can_definition| can_definition.only_block? } - raise Error, "Cannot determine ability conditions from block for #{action.inspect} #{subject.inspect}" - end - relevant.map{|can_definition| - [can_definition.base_behavior, can_definition.conditions(options)] - } - else - false - end - end - - # Returns sql conditions for object, which responds to :sanitize_sql . - # This is useful if you need to generate a database query based on the current ability. - # - # can :manage, User, :id => 1 - # can :manage, User, :manager_id => 1 - # cannot :manage, User, :self_managed => true - # sql_conditions :manage, User # returns not (self_managed = 't') AND ((manager_id = 1) OR (id = 1)) - # - # Normally you will not call this method directly, but instead go through ActiveRecordAdditions#accessible_by method. - # - # If the ability is not defined then false is returned so be sure to take that into consideration. - # If there is just one :can ability, it conditions returned untouched. - # If the ability is defined using a block then this will raise an exception since a hash of conditions cannot be - # determined from that. - def sql_conditions(action, subject, options = {}) - conds = conditions(action, subject, options) - return false if conds == false - return (conds[0][1] || {}) if conds.size==1 && conds[0][0] == true # to match previous spec - - true_cond = subject.send(:sanitize_sql, ['?=?', true, true]) - false_cond = subject.send(:sanitize_sql, ['?=?', true, false]) - conds.reverse.inject(false_cond) do |sql, action| - behavior, condition = action - if condition && condition != {} - condition = subject.send(:sanitize_sql, condition) - case sql - when true_cond - behavior ? true_cond : "not (#{condition})" - when false_cond - behavior ? condition : false_cond - else - behavior ? "(#{condition}) OR (#{sql})" : "not (#{condition}) AND (#{sql})" - end - else - behavior ? true_cond : false_cond - end - end - end - - # Returns the associations used in conditions. This is usually used in the :joins option for a search. - # See ActiveRecordAdditions#accessible_by for use in Active Record. - def association_joins(action, subject) - can_definitions = relevant_can_definitions(action, subject) - unless can_definitions.empty? - if can_definitions.any?{|can_definition| can_definition.only_block? } - raise Error, "Cannot determine association joins from block for #{action.inspect} #{subject.inspect}" - end - collect_association_joins(can_definitions) - else - nil - end + # Returns a CanCan::Query instance to help generate database queries based on the ability. + def query(action, subject, options = {}) + Query.new(relevant_can_definitions_without_block(action, subject), subject, options) end private @@ -288,6 +211,14 @@ module CanCan end end + def relevant_can_definitions_without_block(action, subject) + relevant_can_definitions(action, subject).each do |can_definition| + if can_definition.only_block? + raise Error, "Cannot determine SQL conditions or joins from block for #{action.inspect} #{subject.inspect}" + end + end + end + def default_alias_actions { :read => [:index, :show], @@ -295,33 +226,5 @@ module CanCan :update => [:edit], } end - - def collect_association_joins(can_definitions) - joins = [] - can_definitions.each do |can_definition| - merge_association_joins(joins, can_definition.association_joins || []) - end - joins = clear_association_joins(joins) - joins unless joins.empty? - end - - def merge_association_joins(what, with) - with.each do |join| - name, nested = join.each_pair.first - if at = what.detect{|h| h.has_key?(name) } - at[name] = merge_association_joins(at[name], nested) - else - what << join - end - end - end - - def clear_association_joins(joins) - joins.map do |join| - name, nested = join.each_pair.first - nested.empty? ? name : {name => clear_association_joins(nested)} - end - end - end end diff --git a/lib/cancan/active_record_additions.rb b/lib/cancan/active_record_additions.rb index 31545f2..903aa5f 100644 --- a/lib/cancan/active_record_additions.rb +++ b/lib/cancan/active_record_additions.rb @@ -20,12 +20,12 @@ module CanCan # Here only the articles which the user can update are returned. This # internally uses Ability#conditions method, see that for more information. def accessible_by(ability, action = :read) - conditions = ability.sql_conditions(action, self, :tableize => true) || {:id => nil} - joins = ability.association_joins(action, self) + query = ability.query(action, self, :tableize => true) + conditions = query.sql_conditions || {:id => nil} if respond_to? :where - where(conditions).joins(joins) + where(conditions).joins(query.association_joins) else - scoped(:conditions => conditions, :joins => joins) + scoped(:conditions => conditions, :joins => query.association_joins) end end end diff --git a/lib/cancan/query.rb b/lib/cancan/query.rb new file mode 100644 index 0000000..c2017b7 --- /dev/null +++ b/lib/cancan/query.rb @@ -0,0 +1,119 @@ +module CanCan + + # Generates the sql conditions and association joins for use in ActiveRecord queries. + # Normally you will not use this class directly, but instead through ActiveRecordAdditions#accessible_by. + class Query + def initialize(can_definitions, sanitizer, options) + @can_definitions = can_definitions + @sanitizer = sanitizer + @options = options + end + + # Returns an array of arrays composing from desired action and hash of conditions which match the given ability. + # This is useful if you need to generate a database query based on the current ability. + # + # can :read, Article, :visible => true + # conditions :read, Article # returns [ [ true, { :visible => true } ] ] + # + # can :read, Article, :visible => true + # cannot :read, Article, :blocked => true + # conditions :read, Article # returns [ [ false, { :blocked => true } ], [ true, { :visible => true } ] ] + # + # Normally you will not call this method directly, but instead go through ActiveRecordAdditions#accessible_by method. + # + # If the ability is not defined then false is returned so be sure to take that into consideration. + # If the ability is defined using a block then this will raise an exception since a hash of conditions cannot be + # determined from that. + def conditions + unless @can_definitions.empty? + @can_definitions.map do |can_definition| + [can_definition.base_behavior, can_definition.conditions(@options)] + end + else + false + end + end + + # Returns sql conditions for object, which responds to :sanitize_sql . + # This is useful if you need to generate a database query based on the current ability. + # + # can :manage, User, :id => 1 + # can :manage, User, :manager_id => 1 + # cannot :manage, User, :self_managed => true + # sql_conditions :manage, User # returns not (self_managed = 't') AND ((manager_id = 1) OR (id = 1)) + # + # Normally you will not call this method directly, but instead go through ActiveRecordAdditions#accessible_by method. + # + # If the ability is not defined then false is returned so be sure to take that into consideration. + # If there is just one :can ability, it conditions returned untouched. + # If the ability is defined using a block then this will raise an exception since a hash of conditions cannot be + # determined from that. + def sql_conditions + conds = conditions + return false if conds == false + return (conds[0][1] || {}) if conds.size==1 && conds[0][0] == true # to match previous spec + + true_cond = sanitize_sql(['?=?', true, true]) + false_cond = sanitize_sql(['?=?', true, false]) + conds.reverse.inject(false_cond) do |sql, action| + behavior, condition = action + if condition && condition != {} + condition = sanitize_sql(condition) + case sql + when true_cond + behavior ? true_cond : "not (#{condition})" + when false_cond + behavior ? condition : false_cond + else + behavior ? "(#{condition}) OR (#{sql})" : "not (#{condition}) AND (#{sql})" + end + else + behavior ? true_cond : false_cond + end + end + end + + # Returns the associations used in conditions. This is usually used in the :joins option for a search. + # See ActiveRecordAdditions#accessible_by for use in Active Record. + def association_joins + unless @can_definitions.empty? + collect_association_joins(@can_definitions) + else + nil + end + end + + private + + def sanitize_sql(conditions) + @sanitizer.sanitize_sql(conditions) + end + + def collect_association_joins(can_definitions) + joins = [] + @can_definitions.each do |can_definition| + merge_association_joins(joins, can_definition.association_joins || []) + end + joins = clear_association_joins(joins) + joins unless joins.empty? + end + + def merge_association_joins(what, with) + with.each do |join| + name, nested = join.each_pair.first + if at = what.detect{|h| h.has_key?(name) } + at[name] = merge_association_joins(at[name], nested) + else + what << join + end + end + end + + def clear_association_joins(joins) + joins.map do |join| + name, nested = join.each_pair.first + nested.empty? ? name : {name => clear_association_joins(nested)} + end + end + end +end diff --git a/spec/cancan/ability_spec.rb b/spec/cancan/ability_spec.rb index 57b81c5..1487c2a 100644 --- a/spec/cancan/ability_spec.rb +++ b/spec/cancan/ability_spec.rb @@ -239,100 +239,6 @@ describe CanCan::Ability do @ability.can?(:read, [[4, 5, 6]]).should be_false end - it "should return array of behavior and conditions for a given ability" do - @ability.can :read, Array, :first => 1, :last => 3 - @ability.conditions(:show, Array).should == [[true, {:first => 1, :last => 3}]] - end - - it "should raise an exception when a block is used on condition, and no" do - @ability.can :read, Array do |a| - true - end - lambda { @ability.conditions(:show, Array) }.should raise_error(CanCan::Error, "Cannot determine ability conditions from block for :show Array") - end - - it "should return an array with just behavior for conditions when there are no conditions" do - @ability.can :read, Array - @ability.conditions(:show, Array).should == [ [true, {}] ] - end - - it "should return false when performed on an action which isn't defined" do - @ability.conditions(:foo, Array).should == false - end - - it "should return hash for single `can` definition" do - @ability.can :read, SqlSanitizer, :blocked => false, :user_id => 1 - - @ability.sql_conditions(:read, SqlSanitizer).should == { :blocked => false, :user_id => 1 } - end - - it "should return `sql` for single `can` definition in front of default `cannot` condition" do - @ability.cannot :read, SqlSanitizer - @ability.can :read, SqlSanitizer, :blocked => false, :user_id => 1 - - result = @ability.sql_conditions(:read, SqlSanitizer) - result.should include("blocked=false") - result.should include(" AND ") - result.should include("user_id=1") - end - - it "should return `true condition` for single `can` definition in front of default `can` condition" do - @ability.can :read, SqlSanitizer - @ability.can :read, SqlSanitizer, :blocked => false, :user_id => 1 - - @ability.sql_conditions(:read, SqlSanitizer).should == 'true=true' - end - - it "should return `false condition` for single `cannot` definition" do - @ability.cannot :read, SqlSanitizer, :blocked => true, :user_id => 1 - - @ability.sql_conditions(:read, SqlSanitizer).should == 'true=false' - end - - it "should return `false condition` for single `cannot` definition in front of default `cannot` condition" do - @ability.cannot :read, SqlSanitizer - @ability.cannot :read, SqlSanitizer, :blocked => true, :user_id => 1 - - @ability.sql_conditions(:read, SqlSanitizer).should == 'true=false' - end - - it "should return `not (sql)` for single `cannot` definition in front of default `can` condition" do - @ability.can :read, SqlSanitizer - @ability.cannot :read, SqlSanitizer, :blocked => true, :user_id => 1 - - result = @ability.sql_conditions(:read, SqlSanitizer) - result.should include("not ") - result.should include("blocked=true") - result.should include(" AND ") - result.should include("user_id=1") - end - - it "should return appropriate sql conditions in complex case" do - @ability.can :read, SqlSanitizer - @ability.can :manage, SqlSanitizer, :id => 1 - @ability.can :update, SqlSanitizer, :manager_id => 1 - @ability.cannot :update, SqlSanitizer, :self_managed => true - - @ability.sql_conditions(:update, SqlSanitizer).should == 'not (self_managed=true) AND ((manager_id=1) OR (id=1))' - @ability.sql_conditions(:manage, SqlSanitizer).should == {:id=>1} - @ability.sql_conditions(:read, SqlSanitizer).should == 'true=true' - end - - it "should accept array condition for use in sql" do - @ability.can :read, SqlSanitizer, ["user_id = ?", 1] - - @ability.sql_conditions(:read, SqlSanitizer).should == ['user_id = ?', 1] - @ability.association_joins(:read, SqlSanitizer).should be_nil - end - - it "should accept array condition for use in sql and do sanitizing in complex conditions" do - @ability.cannot :read, SqlSanitizer - @ability.can :read, SqlSanitizer, ["user_id = ?", 1] - - @ability.sql_conditions(:read, SqlSanitizer).should == 'user_id = 1' - @ability.association_joins(:read, SqlSanitizer).should be_nil - end - it "should has eated cheezburger" do lambda { @ability.can? :has, :cheezburger diff --git a/spec/cancan/active_record_additions_spec.rb b/spec/cancan/active_record_additions_spec.rb index fac27d2..14585d3 100644 --- a/spec/cancan/active_record_additions_spec.rb +++ b/spec/cancan/active_record_additions_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" describe CanCan::ActiveRecordAdditions do before(:each) do - @model_class = Class.new(SqlSanitizer) + @model_class = Class.new(Person) stub(@model_class).scoped { :scoped_stub } @model_class.send(:include, CanCan::ActiveRecordAdditions) @ability = Object.new @@ -44,8 +44,8 @@ describe CanCan::ActiveRecordAdditions do stub(@model_class).scoped( :conditions => condition, :joins => joins ) { :found_records } end end - @ability.sql_conditions(:read, @model_class).should == '(too.car=1 AND too.far.bar=1) OR (foo.bar=1)' - @ability.association_joins(:read, @model_class).should == [{:too => [:far]}, :foo] + # @ability.sql_conditions(:read, @model_class).should == '(too.car=1 AND too.far.bar=1) OR (foo.bar=1)' + # @ability.association_joins(:read, @model_class).should == [{:too => [:far]}, :foo] @model_class.accessible_by(@ability).should == :found_records end end diff --git a/spec/cancan/query_spec.rb b/spec/cancan/query_spec.rb new file mode 100644 index 0000000..d3d64e0 --- /dev/null +++ b/spec/cancan/query_spec.rb @@ -0,0 +1,102 @@ +require "spec_helper" + +describe CanCan::Query do + before(:each) do + @ability = Object.new + @ability.extend(CanCan::Ability) + end + + it "should return array of behavior and conditions for a given ability" do + @ability.can :read, Person, :first => 1, :last => 3 + @ability.query(:show, Person).conditions.should == [[true, {:first => 1, :last => 3}]] + end + + it "should raise an exception when a block is used on condition, and no hash" do + @ability.can :read, Person do |a| + true + end + lambda { @ability.query(:show, Person).conditions }.should raise_error(CanCan::Error, "Cannot determine SQL conditions or joins from block for :show Person") + end + + it "should return an array with just behavior for conditions when there are no conditions" do + @ability.can :read, Person + @ability.query(:show, Person).conditions.should == [ [true, {}] ] + end + + it "should return false when performed on an action which isn't defined" do + @ability.query(:foo, Person).conditions.should == false + end + + it "should return hash for single `can` definition" do + @ability.can :read, Person, :blocked => false, :user_id => 1 + + @ability.query(:read, Person).sql_conditions.should == { :blocked => false, :user_id => 1 } + end + + it "should return `sql` for single `can` definition in front of default `cannot` condition" do + @ability.cannot :read, Person + @ability.can :read, Person, :blocked => false, :user_id => 1 + + result = @ability.query(:read, Person).sql_conditions + result.should include("blocked=false") + result.should include(" AND ") + result.should include("user_id=1") + end + + it "should return `true condition` for single `can` definition in front of default `can` condition" do + @ability.can :read, Person + @ability.can :read, Person, :blocked => false, :user_id => 1 + + @ability.query(:read, Person).sql_conditions.should == 'true=true' + end + + it "should return `false condition` for single `cannot` definition" do + @ability.cannot :read, Person, :blocked => true, :user_id => 1 + + @ability.query(:read, Person).sql_conditions.should == 'true=false' + end + + it "should return `false condition` for single `cannot` definition in front of default `cannot` condition" do + @ability.cannot :read, Person + @ability.cannot :read, Person, :blocked => true, :user_id => 1 + + @ability.query(:read, Person).sql_conditions.should == 'true=false' + end + + it "should return `not (sql)` for single `cannot` definition in front of default `can` condition" do + @ability.can :read, Person + @ability.cannot :read, Person, :blocked => true, :user_id => 1 + + result = @ability.query(:read, Person).sql_conditions + result.should include("not ") + result.should include("blocked=true") + result.should include(" AND ") + result.should include("user_id=1") + end + + it "should return appropriate sql conditions in complex case" do + @ability.can :read, Person + @ability.can :manage, Person, :id => 1 + @ability.can :update, Person, :manager_id => 1 + @ability.cannot :update, Person, :self_managed => true + + @ability.query(:update, Person).sql_conditions.should == 'not (self_managed=true) AND ((manager_id=1) OR (id=1))' + @ability.query(:manage, Person).sql_conditions.should == {:id=>1} + @ability.query(:read, Person).sql_conditions.should == 'true=true' + end + + it "should accept array condition for use in sql" do + @ability.can :read, Person, ["user_id = ?", 1] + + @ability.query(:read, Person).sql_conditions.should == ['user_id = ?', 1] + @ability.query(:read, Person).association_joins.should be_nil + end + + it "should accept array condition for use in sql and do sanitizing in complex conditions" do + @ability.cannot :read, Person + @ability.can :read, Person, ["user_id = ?", 1] + + @ability.query(:read, Person).sql_conditions.should == 'user_id = 1' + @ability.query(:read, Person).association_joins.should be_nil + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2654c31..3890748 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -18,12 +18,8 @@ class Ability end end -# this class helps out in testing nesting +# this class helps out in testing nesting and SQL conditions class Person -end - -# Maybe we can use ActiveRecord directly here instead of duplicating the behavior -class SqlSanitizer def self.sanitize_sql(hash_cond) case hash_cond when Hash @@ -34,7 +30,6 @@ class SqlSanitizer end end - private def self.sanitize_hash(hash) hash.map do |name, value| if Hash === value