refactoring much of Ability class into separate CanDefinition class

lib/cancan.rb

@@ -1,4 +1,5 @@
 require 'cancan/ability'
+require 'cancan/can_definition'
 require 'cancan/controller_resource'
 require 'cancan/resource_authorization'
 require 'cancan/controller_additions'

lib/cancan/ability.rb

@@ -48,11 +48,8 @@ module CanCan
     #   end
     # 
     def can?(action, subject, *extra_args)
-      matching_can_definition(action, subject) do |base_behavior, defined_actions, defined_subjects, defined_conditions, defined_block|
+      can_definition = matching_can_definition(action, subject)
-        result = can_perform_action?(action, subject, defined_actions, defined_subjects, defined_conditions, defined_block, extra_args)
+      can_definition && can_definition.can?(action, subject, extra_args)
-        return base_behavior ? result : !result
-      end
-      false
     end
     
     # Convenience method which works the same as "can?" but returns the opposite value.
@@ -117,8 +114,7 @@ module CanCan
     #   can? :read, :stats # => true
     # 
     def can(action, subject, conditions = nil, &block)
-      @can_definitions ||= []
+      can_definitions << CanDefinition.new(true, action, subject, conditions, block)
-      @can_definitions << [true, action, subject, conditions, block]
     end
     
     # Define an ability which cannot be done. Accepts the same arguments as "can".
@@ -134,8 +130,7 @@ module CanCan
     #   end
     # 
     def cannot(action, subject, conditions = nil, &block)
-      @can_definitions ||= []
+      can_definitions << CanDefinition.new(false, action, subject, conditions, block)
-      @can_definitions << [false, action, subject, conditions, block]
     end
     
     # Alias one or more actions into another one.
@@ -195,22 +190,25 @@ 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)
-      matching_can_definition(action, subject) do |base_behavior, defined_actions, defined_subjects, defined_conditions, defined_block|
+      can_definition = matching_can_definition(action, subject)
-        raise Error, "Cannot determine ability conditions from block for #{action.inspect} #{subject.inspect}" if defined_block
+      if can_definition
-        return defined_conditions || {}
+        raise Error, "Cannot determine ability conditions from block for #{action.inspect} #{subject.inspect}" if can_definition.block
-      end
+        can_definition.conditions || {}
+      else
         false
       end
+    end
     
     private
     
-    def matching_can_definition(action, subject, &block)
+    def can_definitions
-      (@can_definitions || []).reverse.each do |base_behavior, defined_action, defined_subject, defined_conditions, defined_block|
+      @can_definitions ||= []
-        defined_actions = expand_actions(defined_action)
-        defined_subjects = [defined_subject].flatten
-        if includes_action?(defined_actions, action) && includes_subject?(defined_subjects, subject)
-          return block.call(base_behavior, defined_actions, defined_subjects, defined_conditions, defined_block)
     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
       end
     end
     
@@ -222,48 +220,6 @@ module CanCan
       }
     end
     
-    def expand_actions(actions)
-      [actions].flatten.map do |action|
-        if aliased_actions[action]
-          [action, *aliased_actions[action]]
-        else
-          action
-        end
-      end.flatten
-    end
-    
-    def can_perform_action?(action, subject, defined_actions, defined_subjects, defined_conditions, defined_block, extra_args)
-      if defined_block
-        block_args = []
-        block_args << action if defined_actions.include?(:manage)
-        block_args << (subject.class == Class ? subject : subject.class) if defined_subjects.include?(:all)
-        block_args << (subject.class == Class ? nil : subject)
-        block_args += extra_args
-        defined_block.call(*block_args)
-      elsif defined_conditions
-        if subject.class == Class
-          true
-        else
-          matches_conditions? subject, defined_conditions
-        end
-      else
-        true
-      end
-    end
-    
-    def matches_conditions?(subject, defined_conditions)
-      defined_conditions.all? do |name, value|
-        attribute = subject.send(name)
-        if value.kind_of?(Hash)
-          matches_conditions? attribute, value
-        elsif value.kind_of?(Array) || value.kind_of?(Range)
-          value.include? attribute
-        else
-          attribute == value
-        end
-      end
-    end
-    
     def includes_action?(actions, action)
       actions.include?(:manage) || actions.include?(action)
     end

lib/cancan/can_definition.rb

@@ -0,0 +1,71 @@
+module CanCan
+  # This class is used internally and should only be called through Ability.
+  class CanDefinition # :nodoc:
+    attr_reader :conditions, :block
+    
+    def initialize(base_behavior, action, subject, conditions, block)
+      @base_behavior = base_behavior
+      @actions = [action].flatten
+      @subjects = [subject].flatten
+      @conditions = conditions
+      @block = block
+    end
+  
+    def expand_actions(aliased_actions)
+      @expanded_actions = @actions.map do |action|
+        aliased_actions[action] ? [action, *aliased_actions[action]] : action
+      end.flatten
+    end
+    
+    def matches?(action, subject)
+      matches_action?(action) && matches_subject?(subject)
+    end
+    
+    def can?(action, subject, extra_args)
+      result = can_without_base_behavior?(action, subject, extra_args)
+      @base_behavior ? result : !result
+    end
+    
+    private
+    
+    def matches_action?(action)
+      @expanded_actions.include?(:manage) || @expanded_actions.include?(action)
+    end
+    
+    def matches_subject?(subject)
+      @subjects.include?(:all) || @subjects.include?(subject) || @subjects.any? { |c| c.kind_of?(Class) && subject.kind_of?(c) }
+    end
+    
+    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
+      else
+        true
+      end
+    end
+    
+    def matches_conditions?(subject, conditions = @conditions)
+      conditions.all? do |name, value|
+        attribute = subject.send(name)
+        if value.kind_of?(Hash)
+          matches_conditions? attribute, value
+        elsif value.kind_of?(Array) || value.kind_of?(Range)
+          value.include? attribute
+        else
+          attribute == value
+        end
+      end
+    end
+    
+    def call_block(action, subject, extra_args)
+      block_args = []
+      block_args << action if @expanded_actions.include?(:manage)
+      block_args << (subject.class == Class ? subject : subject.class) if @subjects.include?(:all)
+      block_args << (subject.class == Class ? nil : subject)
+      block_args += extra_args
+      @block.call(*block_args)
+    end
+  end
+end