aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRick Olson <technoweenie@gmail.com>2007-05-02 13:22:38 +0000
committerRick Olson <technoweenie@gmail.com>2007-05-02 13:22:38 +0000
commit0778464168960a830b806f66a200e86a163a5831 (patch)
tree080835c3e64fb600bce6d80e577a35bc7a5a6fda
parent5a3bb880db43f7bfba3a2838841baaff3d9050af (diff)
downloadrails-0778464168960a830b806f66a200e86a163a5831.tar.gz
rails-0778464168960a830b806f66a200e86a163a5831.tar.bz2
rails-0778464168960a830b806f66a200e86a163a5831.zip
Replace the current block/continuation filter chain handling by an implementation based on a simple loop. #8226 [Stefan Kaes]
git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6649 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
-rw-r--r--actionpack/CHANGELOG2
-rw-r--r--actionpack/lib/action_controller/filters.rb209
-rw-r--r--actionpack/test/controller/filters_test.rb66
3 files changed, 189 insertions, 88 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index f4fdeaa76d..631e328b03 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
*SVN*
+* Replace the current block/continuation filter chain handling by an implementation based on a simple loop. #8226 [Stefan Kaes]
+
* Update UrlWriter to accept :anchor parameter. Closes #6771. [octopod]
* Added RecordTagHelper for using RecordIdentifier conventions on divs and other container elements [DHH]. Example:
diff --git a/actionpack/lib/action_controller/filters.rb b/actionpack/lib/action_controller/filters.rb
index ec5bb759df..3543c80ec9 100644
--- a/actionpack/lib/action_controller/filters.rb
+++ b/actionpack/lib/action_controller/filters.rb
@@ -263,13 +263,13 @@ 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)
+ append_filter_to_chain(filters, :after, &block)
end
# The passed <tt>filters</tt> will be prepended to the array of filters
# that run _after_ actions on this controller are performed.
def prepend_after_filter(*filters, &block)
- append_filter_to_chain(filters, :after, &block)
+ prepend_filter_to_chain(filters, :after, &block)
end
# Shorthand for append_after_filter since it's the most common.
@@ -362,12 +362,12 @@ module ActionController #:nodoc:
# Returns a mapping between filters and the actions that may run them.
def included_actions #:nodoc:
- read_inheritable_attribute("included_actions") || {}
+ @included_actions ||= 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") || {}
+ @excluded_actions ||= read_inheritable_attribute("excluded_actions") || {}
end
# Find a filter in the filter_chain where the filter method matches the _filter_ param
@@ -381,10 +381,11 @@ module ActionController #:nodoc:
# Returns true if the filter is excluded from the given action
def filter_excluded_from_action?(filter,action) #:nodoc:
- if (ia = included_actions[filter]) && !ia.empty?
+ case
+ when ia = included_actions[filter]
!ia.include?(action)
- else
- (excluded_actions[filter] || []).include?(action)
+ when ea = excluded_actions[filter]
+ ea.include?(action)
end
end
@@ -397,20 +398,28 @@ module ActionController #:nodoc:
@filter = filter
end
+ def type
+ :around
+ end
+
def before?
- false
+ type == :before
end
def after?
- false
+ type == :after
end
def around?
- true
+ type == :around
+ end
+
+ def run(controller)
+ raise ActionControllerError, 'No filter type: Nothing to do here.'
end
def call(controller, &block)
- raise(ActionControllerError, 'No filter type: Nothing to do here.')
+ run(controller)
end
end
@@ -420,35 +429,38 @@ module ActionController #:nodoc:
def filter
@filter.filter
end
-
- def around?
- false
- end
end
class BeforeFilterProxy < FilterProxy #:nodoc:
- def before?
- true
+ def type
+ :before
end
- def call(controller, &block)
- if false == @filter.call(controller) # must only stop if equal to false. only filters returning false are halted.
- controller.halt_filter_chain(@filter, :returned_false)
- else
- yield
+ def run(controller)
+ # only filters returning false are halted.
+ if false == @filter.call(controller)
+ controller.halt_filter_chain(@filter)
end
end
+
+ def call(controller)
+ yield unless run(controller)
+ end
end
class AfterFilterProxy < FilterProxy #:nodoc:
- def after?
- true
+ def type
+ :after
end
- def call(controller, &block)
- yield
+ def run(controller)
@filter.call(controller)
end
+
+ def call(controller)
+ yield
+ run(controller)
+ end
end
class SymbolFilter < Filter #:nodoc:
@@ -485,29 +497,67 @@ module ActionController #:nodoc:
end
end
+ class ClassBeforeFilter < Filter #:nodoc:
+ def call(controller, &block)
+ @filter.before(controller)
+ end
+ end
+
+ class ClassAfterFilter < Filter #:nodoc:
+ def call(controller, &block)
+ @filter.after(controller)
+ end
+ end
+
protected
- def append_filter_to_chain(filters, position = :around, &block)
- write_inheritable_array('filter_chain', create_filters(filters, position, &block) )
+ def append_filter_to_chain(filters, filter_type = :around, &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 = :around, &block)
+ pos = find_filter_prepend_position(filters, filter_type)
+ update_filter_chain(filters, filter_type, pos, &block)
+ end
+
+ def update_filter_chain(filters, filter_type, pos, &block)
+ new_filters = create_filters(filters, filter_type, &block)
+ new_chain = filter_chain.insert(pos, new_filters).flatten
+ write_inheritable_attribute('filter_chain', new_chain)
end
- def prepend_filter_to_chain(filters, position = :around, &block)
- write_inheritable_attribute('filter_chain', create_filters(filters, position, &block) + filter_chain)
+ def find_filter_append_position(filters, filter_type)
+ unless filter_type == :after
+ filter_chain.each_with_index do |f,i|
+ return i if f.after?
+ end
+ end
+ return -1
+ end
+
+ def find_filter_prepend_position(filters, filter_type)
+ if filter_type == :after
+ filter_chain.each_with_index do |f,i|
+ return i if f.after?
+ end
+ end
+ return 0
end
- def create_filters(filters, position, &block) #:nodoc:
+ def create_filters(filters, filter_type, &block) #:nodoc:
filters, conditions = extract_conditions(filters, &block)
- filters.map! { |filter| find_or_create_filter(filter,position) }
+ filters.map! { |filter| find_or_create_filter(filter, filter_type) }
update_conditions(filters, conditions)
filters
end
- def find_or_create_filter(filter,position)
- if found_filter = find_filter(filter) { |f| f.send("#{position}?") }
+ def find_or_create_filter(filter, filter_type)
+ if found_filter = find_filter(filter) { |f| f.type == filter_type }
found_filter
else
- f = class_for_filter(filter).new(filter)
+ f = class_for_filter(filter, filter_type).new(filter)
# apply proxy to filter if necessary
- case position
+ case filter_type
when :before
BeforeFilterProxy.new(f)
when :after
@@ -520,7 +570,7 @@ module ActionController #:nodoc:
# The determination of the filter type was once done at run time.
# This method is here to extract as much logic from the filter run time as possible
- def class_for_filter(filter) #:nodoc:
+ def class_for_filter(filter, filter_type) #:nodoc:
case
when filter.is_a?(Symbol)
SymbolFilter
@@ -534,8 +584,12 @@ module ActionController #:nodoc:
end
when filter.respond_to?(:filter)
ClassFilter
+ when filter.respond_to?(:before) && filter_type == :before
+ ClassBeforeFilter
+ when filter.respond_to?(:after) && filter_type == :after
+ ClassAfterFilter
else
- raise(ActionControllerError, 'A filters must be a Symbol, Proc, Method, or object responding to filter.')
+ raise(ActionControllerError, 'A filter must be a Symbol, Proc, Method, or object responding to filter, after or before.')
end
end
@@ -550,8 +604,8 @@ module ActionController #:nodoc:
return if conditions.empty?
if conditions[:only]
write_inheritable_hash('included_actions', condition_hash(filters, conditions[:only]))
- else
- write_inheritable_hash('excluded_actions', condition_hash(filters, conditions[:except])) if conditions[:except]
+ elsif conditions[:except]
+ write_inheritable_hash('excluded_actions', condition_hash(filters, conditions[:except]))
end
end
@@ -576,9 +630,9 @@ module ActionController #:nodoc:
def remove_actions_from_included_actions!(filters,*actions)
actions = actions.flatten.map(&:to_s)
- updated_hash = filters.inject(included_actions) do |hash,filter|
+ updated_hash = filters.inject(read_inheritable_attribute('included_actions')||{}) do |hash,filter|
ia = (hash[filter] || []) - actions
- ia.blank? ? hash.delete(filter) : hash[filter] = ia
+ ia.empty? ? hash.delete(filter) : hash[filter] = ia
hash
end
write_inheritable_attribute('included_actions', updated_hash)
@@ -595,7 +649,9 @@ module ActionController #:nodoc:
def proxy_before_and_after_filter(filter) #:nodoc:
return filter unless filter_responds_to_before_and_after(filter)
Proc.new do |controller, action|
- unless filter.before(controller) == false
+ if filter.before(controller) == false
+ controller.send :halt_filter_chain, filter
+ else
begin
action.call
ensure
@@ -619,30 +675,59 @@ module ActionController #:nodoc:
self.class.filter_chain
end
- def call_filter(chain, index)
- return (performed? || perform_action_without_filters) if index >= chain.size
- filter = chain[index]
- return call_filter(chain, index.next) if self.class.filter_excluded_from_action?(filter,action_name)
-
- halted = false
- filter.call(self) do
- halted = call_filter(chain, index.next)
+ def skip_excluded_filters(chain, index)
+ while (filter = chain[index]) && self.class.filter_excluded_from_action?(filter, action_name)
+ index = index.next
end
- halt_filter_chain(filter.filter, :no_yield) if halted == false unless @before_filter_chain_aborted
- halted
+ [filter, index]
end
- def halt_filter_chain(filter, reason)
- if logger
- case reason
- when :no_yield
- logger.info "Filter chain halted as [#{filter.inspect}] did not yield."
- when :returned_false
- logger.info "Filter chain halted as [#{filter.inspect}] returned false."
+ def call_filters(chain, index, nesting)
+ # run before filters until we find an after filter or around filter
+ while true
+ filter, index = skip_excluded_filters(chain, index)
+ break unless filter
+ case filter.type
+ when :before
+ # invoke before filter
+ filter.run(self)
+ index = index.next
+ break if @before_filter_chain_aborted
+ when :around
+ filter.call(self) do
+ # all remaining before and around filters will be run in this call
+ index = call_filters(chain, index.next, nesting.next)
+ end
+ break
+ else
+ # no before or around filters left
+ break
end
end
+
+ aborted = @before_filter_chain_aborted
+ perform_action_without_filters unless performed? || aborted
+ return index if aborted || nesting != 0
+
+ # run after filters, if any
+ while filter = chain[index]
+ filter, index = skip_excluded_filters(chain, index)
+ case filter.type
+ when :after
+ filter.run(self)
+ index = index.next
+ else
+ raise ActionControllerError, "filter #{filter.inspect} was in the wrong place!"
+ end
+ end
+
+ index.next
+ end
+
+ def halt_filter_chain(filter)
+ logger.info "Filter chain halted as [#{filter.inspect}] returned false." if logger
@before_filter_chain_aborted = true
- return false
+ false
end
protected
@@ -654,7 +739,7 @@ module ActionController #:nodoc:
private
def perform_action_with_filters
- call_filter(self.class.filter_chain, 0)
+ call_filters(self.class.filter_chain, 0, 0)
end
def process_cleanup_with_filters
diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb
index 3a74eba04a..f250b24f00 100644
--- a/actionpack/test/controller/filters_test.rb
+++ b/actionpack/test/controller/filters_test.rb
@@ -14,7 +14,7 @@ class FilterTest < Test::Unit::TestCase
@ran_filter ||= []
@ran_filter << "ensure_login"
end
-
+
def clean_up
@ran_after_filter ||= []
@ran_after_filter << "clean_up"
@@ -62,7 +62,7 @@ class FilterTest < Test::Unit::TestCase
render :inline => "something else"
end
end
-
+
class ConditionalFilterController < ActionController::Base
def show
render :inline => "ran action"
@@ -86,7 +86,7 @@ class FilterTest < Test::Unit::TestCase
@ran_filter ||= []
@ran_filter << "clean_up_tmp"
end
-
+
def rescue_action(e) raise(e) end
end
@@ -94,7 +94,7 @@ class FilterTest < Test::Unit::TestCase
before_filter :ensure_login, :except => [ :show_without_filter, :another_action ]
end
- class OnlyConditionSymController < ConditionalFilterController
+ class OnlyConditionSymController < ConditionalFilterController
before_filter :ensure_login, :only => :show
end
@@ -104,10 +104,10 @@ class FilterTest < Test::Unit::TestCase
class BeforeAndAfterConditionController < ConditionalFilterController
before_filter :ensure_login, :only => :show
- after_filter :clean_up_tmp, :only => :show
+ after_filter :clean_up_tmp, :only => :show
end
-
- class OnlyConditionProcController < ConditionalFilterController
+
+ class OnlyConditionProcController < ConditionalFilterController
before_filter(:only => :show) {|c| c.assigns["ran_proc_filter"] = true }
end
@@ -145,7 +145,7 @@ class FilterTest < Test::Unit::TestCase
class ConditionalSkippingController < TestController
skip_before_filter :ensure_login, :only => [ :login ]
skip_after_filter :clean_up, :only => [ :login ]
-
+
before_filter :find_user, :only => [ :change_password ]
def login
@@ -155,7 +155,7 @@ class FilterTest < Test::Unit::TestCase
def change_password
render :inline => "ran action"
end
-
+
protected
def find_user
@ran_filter ||= []
@@ -166,15 +166,15 @@ class FilterTest < Test::Unit::TestCase
class ConditionalParentOfConditionalSkippingController < ConditionalFilterController
before_filter :conditional_in_parent, :only => [:show, :another_action]
after_filter :conditional_in_parent, :only => [:show, :another_action]
-
+
private
-
+
def conditional_in_parent
@ran_filter ||= []
@ran_filter << 'conditional_in_parent'
end
end
-
+
class ChildOfConditionalParentController < ConditionalParentOfConditionalSkippingController
skip_before_filter :conditional_in_parent, :only => :another_action
skip_after_filter :conditional_in_parent, :only => :another_action
@@ -197,7 +197,7 @@ class FilterTest < Test::Unit::TestCase
controller.assigns["was_audited"] = true
end
end
-
+
class AroundFilter
def before(controller)
@execution_log = "before"
@@ -209,7 +209,7 @@ class FilterTest < Test::Unit::TestCase
controller.assigns["execution_log"] = @execution_log + " and after"
controller.assigns["after_ran"] = true
controller.class.execution_log << " after aroundfilter " if controller.respond_to? :execution_log
- end
+ end
end
class AppendedAroundFilter
@@ -219,12 +219,12 @@ class FilterTest < Test::Unit::TestCase
def after(controller)
controller.class.execution_log << " after appended aroundfilter "
- end
- end
-
+ end
+ end
+
class AuditController < ActionController::Base
before_filter(AuditFilter)
-
+
def show
render_text "hello"
end
@@ -234,6 +234,14 @@ class FilterTest < Test::Unit::TestCase
around_filter AroundFilter.new
end
+ class BeforeAfterClassFilterController < PrependingController
+ begin
+ filter = AroundFilter.new
+ before_filter filter
+ after_filter filter
+ end
+ end
+
class MixedFilterController < PrependingController
cattr_accessor :execution_log
@@ -247,7 +255,7 @@ class FilterTest < Test::Unit::TestCase
after_filter { |c| c.class.execution_log << " after procfilter " }
append_around_filter AppendedAroundFilter.new
end
-
+
class MixedSpecializationController < ActionController::Base
class OutOfOrder < StandardError; end
@@ -292,11 +300,11 @@ class FilterTest < Test::Unit::TestCase
def test_base_class_in_isolation
assert_equal [ ], ActionController::Base.before_filters
end
-
+
def test_prepending_filter
assert_equal [ :wonderful_life, :ensure_login ], PrependingController.before_filters
end
-
+
def test_running_filters
assert_equal %w( wonderful_life ensure_login ), test_process(PrependingController).template.assigns["ran_filter"]
end
@@ -304,11 +312,11 @@ class FilterTest < Test::Unit::TestCase
def test_running_filters_with_proc
assert test_process(ProcController).template.assigns["ran_proc_filter"]
end
-
+
def test_running_filters_with_implicit_proc
assert test_process(ImplicitProcController).template.assigns["ran_proc_filter"]
end
-
+
def test_running_filters_with_class
assert test_process(AuditController).template.assigns["was_audited"]
end
@@ -319,7 +327,7 @@ class FilterTest < Test::Unit::TestCase
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"]
@@ -373,6 +381,12 @@ class FilterTest < Test::Unit::TestCase
assert controller.template.assigns["after_ran"]
end
+ def test_before_after_class_filter
+ controller = test_process(BeforeAfterClassFilterController)
+ assert controller.template.assigns["before_ran"]
+ assert controller.template.assigns["after_ran"]
+ end
+
def test_having_properties_in_around_filter
controller = test_process(AroundFilterController)
assert_equal "before and after", controller.template.assigns["execution_log"]
@@ -381,10 +395,10 @@ class FilterTest < Test::Unit::TestCase
def test_prepending_and_appending_around_filter
controller = test_process(MixedFilterController)
assert_equal " before aroundfilter before procfilter before appended aroundfilter " +
- " after appended aroundfilter after aroundfilter after procfilter ",
+ " after appended aroundfilter after aroundfilter after procfilter ",
MixedFilterController.execution_log
end
-
+
def test_rendering_breaks_filtering_chain
response = test_process(RenderingController)
assert_equal "something else", response.body