Ticket #8226: filter_speedup_filter_chain_ordering_trunk.patch

actionpack/test/controller/filters_test.rb

@@ -131 +131 @@
-    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 EmptyFilterChainController < TestController
+    self.filter_chain.clear
+    def show
+      @action_executed = true
+      render :text => "yawp!"
+    end
+  end
+
-  class PrependingController < TestController
-    prepend_before_filter :wonderful_life
-    # skip_before_filter :fire_flash
@@ -293 +301 @@
-      end
-  end
-
+  class PrependingBeforeAndAfterController < ActionController::Base
+    prepend_before_filter :before_all
+    prepend_after_filter :after_all
+    before_filter :between_before_all_and_after_all
+
+    def before_all
+      @ran_filter ||= []
+      @ran_filter << 'before_all'
+    end
+
+    def after_all
+      @ran_filter ||= []
+      @ran_filter << 'after_all'
+    end
+
+    def between_before_all_and_after_all
+      @ran_filter ||= []
+      @ran_filter << 'between_before_all_and_after_all'
+    end
+    def show
+      render :text => 'hello'
+    end
+  end
+
+  def test_empty_filter_chain
+    assert_equal 0, EmptyFilterChainController.filter_chain.size
+    assert test_process(EmptyFilterChainController).template.assigns['action_executed']
+  end
+
-  def test_added_filter_to_inheritance_graph
-    assert_equal [ :ensure_login ], TestController.before_filters
-  end
@@ -426 +463 @@
-    end
-  end
-
+  def test_running_prepended_before_and_after_filter
+    assert_equal 3, PrependingBeforeAndAfterController.filter_chain.length
+    response = test_process(PrependingBeforeAndAfterController)
+    assert_equal %w( before_all between_before_all_and_after_all after_all ), response.template.assigns["ran_filter"]
+  end
+
-  def test_conditional_skipping_of_filters
-    assert_nil test_process(ConditionalSkippingController, "login").template.assigns["ran_filter"]
-    assert_equal %w( ensure_login find_user ), test_process(ConditionalSkippingController, "change_password").template.assigns["ran_filter"]

actionpack/lib/action_controller/filters.rb

@@ -527 +527 @@
-        end
-
-        def find_filter_append_position(filters, filter_type)
+          # appending an after filter puts it at the end of the call chain
+          # before and around filters goe before the first after filter in the chain
-          unless filter_type == :after
-            filter_chain.each_with_index do |f,i|
-              return i if f.after?
@@ -536 +538 @@
-        end
-
-        def find_filter_prepend_position(filters, filter_type)
+          # prepending a before or around filter puts it at the front of the call chain
+          # after filters go before the first after filter in the chain
-          if filter_type == :after
-            filter_chain.each_with_index do |f,i|
-              return i if f.after?
-            end
+            return -1
-          end
-          return 0
-        end
@@ -684 +689 @@
-
-      def call_filters(chain, index, nesting)
-        # run before filters until we find an after filter or around filter
-true
+chain[index]
-          filter, index = skip_excluded_filters(chain, index)
-
+ # end of call chain reached
-          case filter.type
-          when :before
-            # invoke before filter
@@ -710 +715 @@
-        return index if aborted || nesting != 0
-
-        # run after filters, if any
-filter = 
+
-          filter, index = skip_excluded_filters(chain, index)
+          break unless filter
-          case filter.type
-          when :after
-            filter.run(self)