make sure filters in subclasses with :only or :except conditions are treated like skip_filter calls

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

2 files changed, 42 insertions, 3 deletions

diff --git a/actionpack/lib/action_controller/filters.rb b/actionpack/lib/action_controller/filters.rb
index fc409dfa18..83275bb5a6 100644
--- a/actionpack/lib/action_controller/filters.rb
+++ b/actionpack/lib/action_controller/filters.rb
@@ -248,7 +248,10 @@ module ActionController #:nodoc:
       # The passed <tt>filters</tt> will be appended to the filter_chain and
       # will execute before the action on this controller is performed.
       def append_before_filter(*filters, &block)
-        append_filter_to_chain(filters, :before, &block)
+        new_filters, existing_filters = look_for_existing_filters(filters, :before)
+
+        append_filter_to_chain(new_filters, :before, &block)
+        skip_before_filter(existing_filters) unless existing_filters.empty?
       end
 
       # The passed <tt>filters</tt> will be prepended to the filter_chain and
@@ -263,7 +266,10 @@ module ActionController #:nodoc:
       # The passed <tt>filters</tt> will be appended to the array of filters
       # that run _after_ actions on this controller are performed.
       def append_after_filter(*filters, &block)
-        prepend_filter_to_chain(filters, :after, &block)
+        new_filters, existing_filters = look_for_existing_filters(filters, :after)
+
+        prepend_filter_to_chain(new_filters, :after, &block)
+        skip_after_filter(existing_filters) unless existing_filters.empty?
       end
 
       # The passed <tt>filters</tt> will be prepended to the array of filters
@@ -597,6 +603,27 @@ module ActionController #:nodoc:
             end
           end
         end
+
+        def look_for_existing_filters(filters, which)
+          filters, options = extract_conditions(filters)
+          old_filters = []
+
+          filter_chain.select(&"#{which}?".to_sym).each do |f|
+            old_filters << f.filter if filters.include?(f.filter)
+          end
+          
+          new_filters = filters - old_filters + [options]
+
+          if options[:except]
+            old_filters << { :only => options[:except] }
+          elsif options[:only]
+            old_filters << { :except => options[:only] }
+          else
+            old_filters = []
+          end
+
+          [new_filters, old_filters]
+        end
     end
 
     module InstanceMethods # :nodoc:
diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb
index a098840486..3a74eba04a 100644
--- a/actionpack/test/controller/filters_test.rb
+++ b/actionpack/test/controller/filters_test.rb
@@ -20,7 +20,15 @@ class FilterTest < Test::Unit::TestCase
         @ran_after_filter << "clean_up"
       end
   end
-  
+
+  class ChangingTheRequirementsController < TestController
+    before_filter :ensure_login, :except => [:go_wild]
+
+    def go_wild
+      render :text => "gobble"
+    end
+  end
+
   class TestMultipleFiltersController < ActionController::Base
     before_filter :try_1
     before_filter :try_2
@@ -423,6 +431,10 @@ class FilterTest < Test::Unit::TestCase
     assert_equal %w( conditional_in_parent conditional_in_parent ), test_process(ChildOfConditionalParentController).template.assigns['ran_filter']
   end
 
+  def test_changing_the_requirements
+    assert_equal nil, test_process(ChangingTheRequirementsController, "go_wild").template.assigns['ran_filter']
+  end
+
   private
     def test_process(controller, action = "show")
       request = ActionController::TestRequest.new