diff --git a/lib/cancan/ability.rb b/lib/cancan/ability.rb index 535663e..7e4d6f0 100644 --- a/lib/cancan/ability.rb +++ b/lib/cancan/ability.rb @@ -49,8 +49,12 @@ module CanCan # def can?(action, subject, *extra_args) raise Error, "Nom nom nom. I eated it." if action == :has && subject == :cheezburger - can_definition = matching_can_definition(action, subject) - can_definition && can_definition.can?(action, subject, extra_args) + matching_can_definition(action, subject) do |can_definition| + unless (can = can_definition.can?(action, subject, extra_args)) == :_NOT_MATCHED + return can + end + end + false end # Convenience method which works the same as "can?" but returns the opposite value. @@ -179,11 +183,15 @@ module CanCan @aliased_actions = {} end - # Returns a hash of conditions which match the given ability. This is useful if you need to generate a database - # query based on the current ability. + # 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 { :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. # @@ -191,25 +199,74 @@ module CanCan # 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) - can_definition = matching_can_definition(action, subject) - if can_definition - raise Error, "Cannot determine ability conditions from block for #{action.inspect} #{subject.inspect}" if can_definition.block - can_definition.conditions || {} + matched = matching_can_definition(action, subject) + unless matched.empty? + if matched.any?{|can_definition| can_definition.conditions.nil? && can_definition.block } + raise Error, "Cannot determine ability conditions from block for #{action.inspect} #{subject.inspect}" + end + matched.map{|can_definition| + [can_definition.base_behavior, can_definition.conditions] + } 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) + conds = conditions(action, subject) + 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(nil) do |sql, action| + behavior, condition = action + if condition + condition = "#{subject.send(:sanitize_sql, condition)}" + condition = "not (#{condition})" if behavior == false + else + condition = behavior ? true_cond : false_cond + end + case sql + when nil then condition + when true_cond + behavior ? true_cond : condition + when false_cond + behavior ? condition : false_cond + else + behavior ? "(#{condition}) OR (#{sql})" : "#{condition} AND (#{sql})" + 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_definition = matching_can_definition(action, subject) - if can_definition - raise Error, "Cannot determine association joins from block for #{action.inspect} #{subject.inspect}" if can_definition.block - can_definition.association_joins + can_definitions = matching_can_definition(action, subject) + unless can_definitions.empty? + if can_definitions.any?{|can_definition| can_definition.conditions.nil? && can_definition.block } + raise Error, "Cannot determine association joins from block for #{action.inspect} #{subject.inspect}" + end + collect_association_joins(can_definitions) + else + nil end end - + private def can_definitions @@ -217,9 +274,18 @@ module CanCan end def matching_can_definition(action, subject) - can_definitions.reverse.detect do |can_definition| - can_definition.expand_actions(aliased_actions) - can_definition.matches? action, subject + if block_given? + can_definitions.reverse.each do |can_definition| + can_definition.expand_actions(aliased_actions) + if can_definition.matches? action, subject + yield can_definition + break if can_definition.conditions.nil? && can_definition.block.nil? + end + end + else + matched = [] + matching_can_definition(action, subject){|can_definition| matched << can_definition} + matched end end @@ -230,5 +296,32 @@ 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 + clear_association_joins(joins) + 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 2afa33d..eb0729e 100644 --- a/lib/cancan/active_record_additions.rb +++ b/lib/cancan/active_record_additions.rb @@ -20,7 +20,7 @@ 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.conditions(action, self) || {:id => nil} + conditions = ability.sql_conditions(action, self) || {:id => nil} joins = ability.association_joins(action, self) if respond_to? :where where(conditions).joins(joins) diff --git a/lib/cancan/can_definition.rb b/lib/cancan/can_definition.rb index 9b3b945..0124c6a 100644 --- a/lib/cancan/can_definition.rb +++ b/lib/cancan/can_definition.rb @@ -1,7 +1,7 @@ module CanCan # This class is used internally and should only be called through Ability. class CanDefinition # :nodoc: - attr_reader :conditions, :block + attr_reader :conditions, :block, :base_behavior, :definitive def initialize(base_behavior, action, subject, conditions, block) @base_behavior = base_behavior @@ -23,18 +23,19 @@ module CanCan def can?(action, subject, extra_args) result = can_without_base_behavior?(action, subject, extra_args) + return :_NOT_MATCHED if result == :_NOT_MATCHED || !result @base_behavior ? result : !result end def association_joins(conditions = @conditions) joins = [] - conditions.each do |name, value| + (conditions || []).each do |name, value| if value.kind_of? Hash nested = association_joins(value) if nested joins << {name => nested} else - joins << name + joins << {name => []} end end end @@ -54,8 +55,10 @@ module CanCan def can_without_base_behavior?(action, subject, extra_args) if @block call_block(action, subject, extra_args) - elsif @conditions && subject.class != Class - matches_conditions? subject + elsif @conditions.kind_of?(Hash) && subject.class != Class + matches_conditions?(subject) + elsif [true, false, :_NOT_MATCHES].include? @conditions + @conditions else true end diff --git a/spec/cancan/ability_spec.rb b/spec/cancan/ability_spec.rb index 169c040..26c2b76 100644 --- a/spec/cancan/ability_spec.rb +++ b/spec/cancan/ability_spec.rb @@ -16,13 +16,27 @@ describe CanCan::Ability do @ability.can?(:foodfight, String).should be_false end - it "should return what block returns on a can call" do + it "should return what block returns on a can call, except for nil and false" do + @ability.can :read, :all + @ability.can :read, Symbol do |sym| + [ sym ] + end + @ability.can?(:read, Symbol).should == [ nil ] + @ability.can?(:read, :some_symbol).should == [ :some_symbol ] + end + + it "should pass to previous can definition, if block returns false or nil" do @ability.can :read, :all @ability.can :read, Symbol do |sym| sym end - @ability.can?(:read, Symbol).should be_nil + @ability.can :read, Integer do |i| + i > 0 + end + @ability.can?(:read, Symbol).should be_true @ability.can?(:read, :some_symbol).should == :some_symbol + @ability.can?(:read, 1).should be_true + @ability.can?(:read, -1).should be_true end it "should pass class with object if :all objects are accepted" do @@ -121,6 +135,30 @@ describe CanCan::Ability do @ability.can?(:read, 123).should be_false end + it "should pass to previous can definition, if block returns false or nil" do + #same as previous + @ability.can :read, :all + @ability.cannot :read, Integer do |int| + int > 10 ? nil : ( int > 5 ) + end + @ability.can?(:read, "foo").should be_true + @ability.can?(:read, 3).should be_true + @ability.can?(:read, 8).should be_false + @ability.can?(:read, 123).should be_true + + end + + it "should pass to previous cannot definition, if block returns false or nil" do + @ability.cannot :read, :all + @ability.can :read, Integer do |int| + int > 10 ? nil : ( int > 5 ) + end + @ability.can?(:read, "foo").should be_false + @ability.can?(:read, 3).should be_false + @ability.can?(:read, 10).should be_true + @ability.can?(:read, 123).should be_false + end + it "should append aliased actions" do @ability.alias_action :update, :to => :modify @ability.alias_action :destroy, :to => :modify @@ -174,27 +212,48 @@ describe CanCan::Ability do @ability.can?(:read, [[4, 5, 6]]).should be_false end - it "should return conditions for a given ability" do + 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 == {: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" do + 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 empty hash for conditions when there are no conditions" do + 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 == {} + @ability.conditions(:show, Array).should == [ [true, nil] ] 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 appropriate sql conditions" do + obj = Class.new do + def self.sanitize_sql(hash_cond) + case hash_cond + when Hash then hash_cond.map{|name, value| "#{name}=#{value}"} + when Array + hash_cond.shift.gsub('?'){"#{hash_cond.shift.inspect}"} + when String then hash_cond + end + end + end + @ability.can :read, obj + @ability.can :manage, obj, :id => 1 + @ability.can :update, obj, :manager_id => 1 + @ability.cannot :update, obj, :self_managed => true + + @ability.sql_conditions(:update, obj).should == 'not (self_managed=true) AND ((manager_id=1) OR (id=1))' + @ability.sql_conditions(:manage, obj).should == {:id=>1} + @ability.sql_conditions(:read, obj).should == 'true=true' + end + it "should has eated cheezburger" do lambda { @ability.can? :has, :cheezburger diff --git a/spec/cancan/can_definition_spec.rb b/spec/cancan/can_definition_spec.rb index 697dc74..3a4cb24 100644 --- a/spec/cancan/can_definition_spec.rb +++ b/spec/cancan/can_definition_spec.rb @@ -17,7 +17,7 @@ describe CanCan::CanDefinition do it "should return single association for joins" do @conditions[:foo] = {:bar => 1} - @can.association_joins.should == [:foo] + @can.association_joins.should == [{:foo=>[]}] end it "should return multiple associations for joins" do @@ -28,6 +28,6 @@ describe CanCan::CanDefinition do it "should return nested associations for joins" do @conditions[:foo] = {:bar => {1 => 2}} - @can.association_joins.should == [{:foo => [:bar]}] + @can.association_joins.should == [{:foo => [{:bar=>[]}]}] end end