Changeset 9055

Timestamp:

03/18/08 17:56:05 (5 months ago)

Author:

bitsweat

Message:

Refactor filters to use Active Support callbacks. Closes #11235.

Files:

• trunk/actionpack/CHANGELOG (modified) (1 diff)
• trunk/actionpack/lib/action_controller/dispatcher.rb (modified) (1 diff)
• trunk/actionpack/lib/action_controller/filters.rb (modified) (12 diffs)
• trunk/actionpack/test/controller/dispatcher_test.rb (modified) (1 diff)
• trunk/actionpack/test/controller/filters_test.rb (modified) (4 diffs)
• trunk/activerecord/lib/active_record/validations.rb (modified) (1 diff)
• trunk/activesupport/lib/active_support/callbacks.rb (modified) (5 diffs)
• trunk/activesupport/test/callbacks_test.rb (modified) (1 diff)

trunk/actionpack/CHANGELOG

@@ -1 +1 @@
-*SVN*
+
+* Refactor filters to use Active Support callbacks.  #11235 [Josh Peek]
-
-* Fixed that polymorphic routes would modify the input array #11363 [thomas.lee]

trunk/actionpack/lib/action_controller/dispatcher.rb

@@ -21 +21 @@
-      # code adding a preparation block may be reloaded.
-      def to_prepare(identifier = nil, &block)
-[]
+ActiveSupport::Callbacks::CallbackChain.new
-        callback = ActiveSupport::Callbacks::Callback.new(:prepare_dispatch, block, :identifier => identifier)
-
-
-
-
-
-
-
-
-
+
-      end
-

trunk/actionpack/lib/action_controller/filters.rb

@@ -245 +245 @@
-    # the second half of #around and will still run but #after and the
-    # action will not. If #around fails to yield, #after will not be run.
+
+    class FilterChain < ActiveSupport::Callbacks::CallbackChain #:nodoc:
+      def append_filter_to_chain(filters, filter_type, &block)
+        pos = find_filter_append_position(filters, filter_type)
+        update_filter_chain(filters, filter_type, pos, &block)
+      end
+
+      def prepend_filter_to_chain(filters, filter_type, &block)
+        pos = find_filter_prepend_position(filters, filter_type)
+        update_filter_chain(filters, filter_type, pos, &block)
+      end
+
+      def create_filters(filters, filter_type, &block)
+        filters, conditions = extract_options(filters, &block)
+        filters.map! { |filter| find_or_create_filter(filter, filter_type, conditions) }
+        filters
+      end
+
+      def skip_filter_in_chain(*filters, &test)
+        filters, conditions = extract_options(filters)
+        update_filter_in_chain(filters, :skip => conditions, &test)
+      end
+
+      private
+        def update_filter_chain(filters, filter_type, pos, &block)
+          new_filters = create_filters(filters, filter_type, &block)
+          insert(pos, new_filters).flatten!
+        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 go before the first after filter in the chain
+          unless filter_type == :after
+            each_with_index do |f,i|
+              return i if f.after?
+            end
+          end
+          return -1
+        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
+            each_with_index do |f,i|
+              return i if f.after?
+            end
+            return -1
+          end
+          return 0
+        end
+
+        def find_or_create_filter(filter, filter_type, options = {})
+          update_filter_in_chain([filter], options)
+
+          if found_filter = find_callback(filter) { |f| f.type == filter_type }
+            found_filter
+          else
+            filter_kind = case
+            when filter.respond_to?(:before) && filter_type == :before
+              :before
+            when filter.respond_to?(:after) && filter_type == :after
+              :after
+            else
+              :filter
+            end
+
+            case filter_type
+            when :before
+              BeforeFilter.new(filter_kind, filter, options)
+            when :after
+              AfterFilter.new(filter_kind, filter, options)
+            else
+              AroundFilter.new(filter_kind, filter, options)
+            end
+          end
+        end
+
+        def update_filter_in_chain(filters, options, &test)
+          filters.map! { |f| block_given? ? find_callback(f, &test) : find_callback(f) }
+          filters.compact!
+
+          map! do |filter|
+            if filters.include?(filter)
+              new_filter = filter.dup
+              new_filter.options.merge!(options)
+              new_filter
+            else
+              filter
+            end
+          end
+        end
+    end
+
+    class Filter < ActiveSupport::Callbacks::Callback #:nodoc:
+      def before?
+        self.class == BeforeFilter
+      end
+
+      def after?
+        self.class == AfterFilter
+      end
+
+      def around?
+        self.class == AroundFilter
+      end
+
+      private
+        def should_not_skip?(controller)
+          if options[:skip]
+            !included_in_action?(controller, options[:skip])
+          else
+            true
+          end
+        end
+
+        def included_in_action?(controller, options)
+          if options[:only]
+            Array(options[:only]).map(&:to_s).include?(controller.action_name)
+          elsif options[:except]
+            !Array(options[:except]).map(&:to_s).include?(controller.action_name)
+          else
+            true
+          end
+        end
+
+        def should_run_callback?(controller)
+          should_not_skip?(controller) && included_in_action?(controller, options) && super
+        end
+    end
+
+    class AroundFilter < Filter #:nodoc:
+      def type
+        :around
+      end
+
+      def call(controller, &block)
+        if should_run_callback?(controller)
+          proc = filter_responds_to_before_and_after? ? around_proc : method
+          evaluate_method(proc, controller, &block)
+        else
+          block.call
+        end
+      end
+
+      private
+        def filter_responds_to_before_and_after?
+          method.respond_to?(:before) && method.respond_to?(:after)
+        end
+
+        def around_proc
+          Proc.new do |controller, action|
+            method.before(controller)
+
+            if controller.send!(:performed?)
+              controller.send!(:halt_filter_chain, method, :rendered_or_redirected)
+            else
+              begin
+                action.call
+              ensure
+                method.after(controller)
+              end
+            end
+          end
+        end
+    end
+
+    class BeforeFilter < Filter #:nodoc:
+      def type
+        :before
+      end
+
+      def call(controller, &block)
+        super
+        if controller.send!(:performed?)
+          controller.send!(:halt_filter_chain, method, :rendered_or_redirected)
+        end
+      end
+    end
+
+    class AfterFilter < Filter #:nodoc:
+      def type
+        :after
+      end
+    end
+
-    module ClassMethods
-      # 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)
-
+filter_chain.
-      end
-
@@ -255 +441 @@
-      # will execute before the action on this controller is performed.
-      def prepend_before_filter(*filters, &block)
-
+filter_chain.
-      end
-
@@ -264 +450 @@
-      # that run _after_ actions on this controller are performed.
-      def append_after_filter(*filters, &block)
-
+filter_chain.
-      end
-
@@ -270 +456 @@
-      # that run _after_ actions on this controller are performed.
-      def prepend_after_filter(*filters, &block)
-
+filter_chain.
-      end
-
-      # Shorthand for append_after_filter since it's the most common.
-      alias :after_filter :append_after_filter
-
-
-      # If you append_around_filter A.new, B.new, the filter chain looks like
@@ -288 +473 @@
-      # are the code before and after the yield.
-      def append_around_filter(*filters, &block)
-
-
-
-
+
-      end
-
@@ -305 +487 @@
-      # are the code before and after the yield.
-      def prepend_around_filter(*filters, &block)
-
-
-
-
+
-      end
-
@@ -321 +500 @@
-      # just like when you apply the filters.
-      def skip_before_filter(*filters)
-
+filter_chain.
-      end
-
@@ -331 +510 @@
-      # just like when you apply the filters.
-      def skip_after_filter(*filters)
-
+filter_chain.
-      end
-
@@ -341 +520 @@
-      # just like when you apply the filters.
-      def skip_filter(*filters)
-
+filter_chain.
-      end
-
-      # Returns an array of Filter objects for this controller.
-      def filter_chain
-
+
+
+
+
+
+
-      end
-
@@ -352 +536 @@
-      # This method returns the actual filter that was assigned in the controller to maintain existing functionality.
-      def before_filters #:nodoc:
-filter
+method
-      end
-
@@ -358 +542 @@
-      # This method returns the actual filter that was assigned in the controller to maintain existing functionality.
-      def after_filters #:nodoc:
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
-    end
-
@@ -682 +555 @@
-
-      protected
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
-
-      private
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-            index = index.next
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
-    end
-  end

trunk/actionpack/test/controller/dispatcher_test.rb

@@ -12 +12 @@
-    ENV['REQUEST_METHOD'] = 'GET'
-
-[]
+ActiveSupport::Callbacks::CallbackChain.new
-    @dispatcher = Dispatcher.new(@output)
-  end

trunk/actionpack/test/controller/filters_test.rb

@@ -135 +135 @@
-  end
-
+  class ConditionalOptionsFilter < ConditionalFilterController
+    before_filter :ensure_login, :if => Proc.new { |c| true }
+    before_filter :clean_up_tmp, :if => Proc.new { |c| false }
+  end
+
-  class EmptyFilterChainController < TestController
-    self.filter_chain.clear
@@ -467 +472 @@
-  end
-
+  def test_running_conditional_options
+    response = test_process(ConditionalOptionsFilter)
+    assert_equal %w( ensure_login ), response.template.assigns["ran_filter"]
+  end
+
-  def test_running_collection_condition_filters
-    assert_equal %w( ensure_login ), test_process(ConditionalCollectionFilterController).template.assigns["ran_filter"]
@@ -498 +508 @@
-    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
-    bad_filter_controller = Class.new(ActionController::Base)
-    assert_raises(ActionController::ActionControllerError) do
-      bad_filter_controller.before_filter 2
-    end
-  end
-
@@ -747 +750 @@
-  end
-
-  def test_wrong_filter_type
-    assert_raise ArgumentError do
-      Class.new PostsController do
-        around_filter lambda { yield }
-      end
-    end
-  end
-
-  def test_base
-    controller = PostsController

trunk/activerecord/lib/active_record/validations.rb

@@ -283 +283 @@
-      base.send :include, ActiveSupport::Callbacks
-
-
-
+
-        base.class_eval <<-"end_eval"
-          def self.#{validation_method}(*methods, &block)
-
-
-
-
-
+
+
-          end
-
-          def self.#{validation_method}_callback_chain
-
+
+
+
+
+
+
-          end
-        end_eval

trunk/activesupport/lib/active_support/callbacks.rb

@@ -77 +77 @@
-  #   saved
-  module Callbacks
-
-
-
+
+
+
+
+
+
+
+
+
-
-        unless block_given?
-callbacks.
+
-        else
-callbacks.
+
-            result = callback.call(object)
-            break result if terminator.call(result, object)
@@ -91 +97 @@
-      end
-
+      def find_callback(callback, &block)
+        select { |c| c == callback && (!block_given? || yield(c)) }.first
+      end
+
+      def replace_or_append_callback(callback)
+        if found_callback = find_callback(callback)
+          index = index(found_callback)
+          self[index] = callback
+        else
+          self << callback
+        end
+      end
+
+      private
+        def self.extract_options(*methods, &block)
+          methods.flatten!
+          options = methods.extract_options!
+          methods << block if block_given?
+          return methods, options
+        end
+
+        def extract_options(*methods, &block)
+          self.class.extract_options(*methods, &block)
+        end
+    end
+
+    class Callback
-      attr_reader :kind, :method, :identifier, :options
-
@@ -100 +133 @@
-      end
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-      end
-
-      private
-
+, &block
-          case method
-            when Symbol
-
+, &block
-            when String
-              eval(method, object.instance_eval { binding })
-            when Proc, Method
-
+
+
+
+
+
+
+
+
-            else
-              if method.respond_to?(kind)
-
+, &block
-              else
-                raise ArgumentError,
@@ -144 +205 @@
-          class_eval <<-"end_eval"
-            def self.#{callback}(*methods, &block)