diff options
Diffstat (limited to 'actionpack')
| -rw-r--r-- | actionpack/CHANGELOG | 2 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/filters.rb | 209 | ||||
| -rw-r--r-- | actionpack/test/controller/filters_test.rb | 66 |
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 |