Added conditional filters #431 [Marcel]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@354 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 213 insertions, 16 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index 4467f352bb..f4ed8f3bac 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,17 @@
 *SVN*
 
+* Added conditional filters #431 [Marcel]. Example:
+
+    class JournalController < ActionController::Base
+      # only require authentication if the current action is edit or delete
+      before_filter :authorize, :only_on => [ :edit, :delete ]
+    
+      private
+        def authorize
+          # redirect to login unless authenticated
+        end
+    end
+
 * Added authentication framework to protect actions behind a condition and redirect on failure. See ActionController::Authentication for more.
 
 * Added Base#render_nothing as a cleaner way of doing render_text "" when you're not interested in returning anything but an empty response.
diff --git a/actionpack/lib/action_controller/filters.rb b/actionpack/lib/action_controller/filters.rb
index 1ae2ea2251..85df451501 100644
--- a/actionpack/lib/action_controller/filters.rb
+++ b/actionpack/lib/action_controller/filters.rb
@@ -126,18 +126,46 @@ module ActionController #:nodoc:
     #       report_result
     #     end
     #   end
+    #
+    # == Filter conditions
+    #
+    # Filters can be limited to run for only specific actions. This can be expressed either by listing the actions to
+    # exclude or the actions to include when executing the filter. Available conditions are +:only+ or +:except+, both 
+    # of which accept an arbitrary number of method references. For example:
+    #
+    #   class Journal < ActionController::Base
+    #     # only require authentication if the current action is edit or delete
+    #     before_filter :authorize, :only => [ :edit, :delete ]
+    #    
+    #     private
+    #       def authorize
+    #         # redirect to login unless authenticated
+    #       end
+    #   end
+    # 
+    # When setting conditions on inline method (proc) filters the condition must come first and be placed in parenthesis.
+    #
+    #   class UserPreferences < ActionController::Base
+    #     before_filter(:except => :new) { # some proc ... }
+    #     # ...
+    #   end
+    #
     module ClassMethods
       # The passed <tt>filters</tt> will be appended to the array of filters that's run _before_ actions
       # on this controller are performed.
       def append_before_filter(*filters, &block)
+        conditions = extract_conditions!(filters)
         filters << block if block_given?
+        add_action_conditions(filters, conditions)
         append_filter_to_chain("before", filters)
       end
 
       # The passed <tt>filters</tt> will be prepended to the array of filters that's run _before_ actions
       # on this controller are performed.
       def prepend_before_filter(*filters, &block)
+        conditions = extract_conditions!(filters) 
         filters << block if block_given?
+        add_action_conditions(filters, conditions)
         prepend_filter_to_chain("before", filters)
       end
 
@@ -147,14 +175,18 @@ module ActionController #:nodoc:
       # The passed <tt>filters</tt> will be appended to the array of filters that's run _after_ actions
       # on this controller are performed.
       def append_after_filter(*filters, &block)
+        conditions = extract_conditions!(filters) 
         filters << block if block_given?
+        add_action_conditions(filters, conditions)
         append_filter_to_chain("after", filters)
       end
 
       # The passed <tt>filters</tt> will be prepended to the array of filters that's run _after_ actions
       # on this controller are performed.
       def prepend_after_filter(*filters, &block)
+        conditions = extract_conditions!(filters) 
         filters << block if block_given?
+        add_action_conditions(filters, conditions)
         prepend_filter_to_chain("after", filters)
       end
 
@@ -169,8 +201,8 @@ module ActionController #:nodoc:
       #     A#before
       #     A#after
       #   B#after
-      def append_around_filter(filters)
-        for filter in [filters].flatten
+      def append_around_filter(*filters)
+        for filter in filters.flatten
           ensure_filter_responds_to_before_and_after(filter)
           append_before_filter { |c| filter.before(c) }
           prepend_after_filter { |c| filter.after(c) }
@@ -185,8 +217,8 @@ module ActionController #:nodoc:
       #     B#before
       #     B#after
       #   A#after
-      def prepend_around_filter(filters)
-        for filter in [filters].flatten
+      def prepend_around_filter(*filters)
+        for filter in filters.flatten
           ensure_filter_responds_to_before_and_after(filter)
           prepend_before_filter { |c| filter.before(c) }
           append_after_filter   { |c| filter.after(c) }
@@ -206,6 +238,16 @@ module ActionController #:nodoc:
         read_inheritable_attribute("after_filters")
       end
       
+      # Returns a mapping between filters and the actions that may run them.
+      def included_actions #:nodoc:
+        read_inheritable_attribute("included_actions") || {}
+      end
+      
+      # Returns a mapping between filters and actions that may not run them.
+      def excluded_actions #:nodoc:
+        read_inheritable_attribute("excluded_actions") || {}
+      end
+      
       private
         def append_filter_to_chain(condition, filters)
           write_inheritable_array("#{condition}_filters", filters)
@@ -220,6 +262,22 @@ module ActionController #:nodoc:
             raise ActionControllerError, "Filter object must respond to both before and after"
           end
         end
+
+        def extract_conditions!(filters)
+          return nil unless filters.last.is_a? Hash
+          filters.pop
+        end
+
+        def add_action_conditions(filters, conditions)
+          return unless conditions
+          included, excluded = conditions[:only], conditions[:except]
+          write_inheritable_hash("included_actions", condition_hash(filters, included)) && return if included
+          write_inheritable_hash("excluded_actions", condition_hash(filters, excluded)) if excluded
+        end
+
+        def condition_hash(filters, *actions)
+          filters.inject({}) {|hash, filter| hash.merge(filter => actions.flatten.map {|action| action.to_s})}
+        end
     end
 
     module InstanceMethods # :nodoc:
@@ -252,18 +310,21 @@ module ActionController #:nodoc:
       private
         def call_filters(filters)
           filters.each do |filter| 
-            if Symbol === filter
-              if self.send(filter) == false then return false end
-            elsif filter_block?(filter)
-              if filter.call(self) == false then return false end
-            elsif filter_class?(filter)
-              if filter.filter(self) == false then return false end
-            else
-              raise(
-                ActionControllerError, 
-                "Filters need to be either a symbol, proc/method, or class implementing a static filter method"
-              )
+            next if action_exempted?(filter)
+            filter_result = case
+              when filter.is_a?(Symbol)
+                self.send(filter)
+              when filter_block?(filter)
+                filter.call(self)
+              when filter_class?(filter)
+                filter.filter(self)
+              else
+                raise(
+                  ActionControllerError, 
+                  "Filters need to be either a symbol, proc/method, or class implementing a static filter method"
+                )
             end
+            return false if filter_result == false
           end
         end
         
@@ -274,6 +335,15 @@ module ActionController #:nodoc:
         def filter_class?(filter)
           filter.respond_to?("filter")
         end
+
+        def action_exempted?(filter)
+          case
+            when self.class.included_actions[filter]
+              !self.class.included_actions[filter].include?(action_name)
+            when self.class.excluded_actions[filter] 
+              self.class.excluded_actions[filter].include?(action_name)
+          end
+        end
     end
   end
 end
diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb
index ae079c8624..58490aebba 100644
--- a/actionpack/test/controller/filters_test.rb
+++ b/actionpack/test/controller/filters_test.rb
@@ -15,6 +15,74 @@ class FilterTest < Test::Unit::TestCase
       end
   end
   
+  class ConditionalFilterController < ActionController::Base
+    def show
+      render_text "ran action"
+    end
+
+    def another_action
+      render_text "ran action"
+    end
+
+    def show_without_filter
+      render_text "ran action without filter"
+    end
+
+    private
+      def ensure_login
+        @ran_filter ||= []
+        @ran_filter << "ensure_login"
+      end
+
+      def clean_up_tmp
+        @ran_filter ||= []
+        @ran_filter << "clean_up_tmp"
+      end
+      
+      def rescue_action(e) raise(e) end
+  end
+
+  class ConditionalCollectionFilterController < ConditionalFilterController
+    before_filter :ensure_login, :except => [ :show_without_filter, :another_action ]
+  end
+
+  class OnlyConditionSymController < ConditionalFilterController 
+    before_filter :ensure_login, :only => :show
+  end
+
+  class ExceptConditionSymController < ConditionalFilterController
+    before_filter :ensure_login, :except => :show_without_filter
+  end
+
+  class BeforeAndAfterConditionController < ConditionalFilterController
+    before_filter :ensure_login, :only => :show
+    after_filter  :clean_up_tmp, :only => :show 
+  end
+  
+  class OnlyConditionProcController < ConditionalFilterController 
+    before_filter(:only => :show) {|c| c.assigns["ran_proc_filter"] = true }
+  end
+
+  class ExceptConditionProcController < ConditionalFilterController
+    before_filter(:except => :show_without_filter) {|c| c.assigns["ran_proc_filter"] = true }
+  end
+
+  class ConditionalClassFilter
+    def self.filter(controller) controller.assigns["ran_class_filter"] = true end
+  end
+
+  class OnlyConditionClassController < ConditionalFilterController
+    before_filter ConditionalClassFilter, :only => :show
+  end
+
+  class ExceptConditionClassController < ConditionalFilterController
+    before_filter ConditionalClassFilter, :except => :show_without_filter
+  end
+
+  class AnomolousYetValidConditionController < ConditionalFilterController
+    before_filter(ConditionalClassFilter, :ensure_login, Proc.new {|c| c.assigns["ran_proc_filter1"] = true }, :except => :show_without_filter) { |c| c.assigns["ran_proc_filter2"] = true}
+  end
+
   class PrependingController < TestController
     prepend_before_filter :wonderful_life
 
@@ -126,6 +194,53 @@ class FilterTest < Test::Unit::TestCase
     assert test_process(AuditController).template.assigns["was_audited"]
   end
 
+  def test_running_anomolous_yet_valid_condition_filters
+    response = test_process(AnomolousYetValidConditionController)
+    assert_equal %w( ensure_login ), response.template.assigns["ran_filter"]
+    assert response.template.assigns["ran_class_filter"]
+    assert response.template.assigns["ran_proc_filter1"]
+    assert response.template.assigns["ran_proc_filter2"]
+    
+    response = test_process(AnomolousYetValidConditionController, "show_without_filter")
+    assert_equal nil, response.template.assigns["ran_filter"]
+    assert !response.template.assigns["ran_class_filter"]
+    assert !response.template.assigns["ran_proc_filter1"]
+    assert !response.template.assigns["ran_proc_filter2"]
+  end
+
+  def test_running_collection_condition_filters
+    assert_equal %w( ensure_login ), test_process(ConditionalCollectionFilterController).template.assigns["ran_filter"]
+    assert_equal nil, test_process(ConditionalCollectionFilterController, "show_without_filter").template.assigns["ran_filter"]
+    assert_equal nil, test_process(ConditionalCollectionFilterController, "another_action").template.assigns["ran_filter"]
+  end
+
+  def test_running_only_condition_filters
+    assert_equal %w( ensure_login ), test_process(OnlyConditionSymController).template.assigns["ran_filter"]
+    assert_equal nil, test_process(OnlyConditionSymController, "show_without_filter").template.assigns["ran_filter"]
+
+    assert test_process(OnlyConditionProcController).template.assigns["ran_proc_filter"]
+    assert !test_process(OnlyConditionProcController, "show_without_filter").template.assigns["ran_proc_filter"]
+
+    assert test_process(OnlyConditionClassController).template.assigns["ran_class_filter"]
+    assert !test_process(OnlyConditionClassController, "show_without_filter").template.assigns["ran_class_filter"]
+  end
+
+  def test_running_except_condition_filters
+    assert_equal %w( ensure_login ), test_process(ExceptConditionSymController).template.assigns["ran_filter"]
+    assert_equal nil, test_process(ExceptConditionSymController, "show_without_filter").template.assigns["ran_filter"]
+
+    assert test_process(ExceptConditionProcController).template.assigns["ran_proc_filter"]
+    assert !test_process(ExceptConditionProcController, "show_without_filter").template.assigns["ran_proc_filter"]
+
+    assert test_process(ExceptConditionClassController).template.assigns["ran_class_filter"]
+    assert !test_process(ExceptConditionClassController, "show_without_filter").template.assigns["ran_class_filter"]
+  end
+
+  def test_running_before_and_after_condition_filters
+    assert_equal %w( ensure_login clean_up_tmp), test_process(BeforeAndAfterConditionController).template.assigns["ran_filter"]
+    assert_equal nil, test_process(BeforeAndAfterConditionController, "show_without_filter").template.assigns["ran_filter"]
+  end
+  
   def test_bad_filter
     assert_raises(ActionController::ActionControllerError) { 
       test_process(BadFilterController)
@@ -156,4 +271,4 @@ class FilterTest < Test::Unit::TestCase
       request.action = action
       controller.process(request, ActionController::TestResponse.new)
     end
-end
\ No newline at end of file
+end