mirror of
https://github.com/github/rails.git
synced 2026-01-30 00:38:00 -05:00
r5540@ks: jeremy | 2006-10-08 23:05:30 -0700
#5949 r5541@ks: jeremy | 2006-10-08 23:07:08 -0700 Fix filter skipping in controller subclasses. r5557@ks: jeremy | 2006-10-08 23:11:24 -0700 Update changelog. Closes #5949, references #6297, references #6299. git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@5268 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
This commit is contained in:
@@ -1,5 +1,7 @@
|
||||
*SVN*
|
||||
|
||||
* Fix filter skipping in controller subclasses. #5949, #6297, #6299 [Martin Emde]
|
||||
|
||||
* render_text may optionally append to the response body. render_javascript appends by default. This allows you to chain multiple render :update calls by setting @performed_render = false between them (awaiting a better public API). [Jeremy Kemper]
|
||||
|
||||
* Rename test assertion to prevent shadowing. Closes #6306. [psross]
|
||||
|
||||
@@ -362,12 +362,12 @@ module ActionController #:nodoc:
|
||||
|
||||
# Returns a mapping between filters and the actions that may run them.
|
||||
def included_actions #:nodoc:
|
||||
filter_chain.inject({}) { |h,f| h[f.filter] = f.included_actions; h }
|
||||
read_inheritable_attribute("included_actions") || {}
|
||||
end
|
||||
|
||||
# Returns a mapping between filters and actions that may not run them.
|
||||
def excluded_actions #:nodoc:
|
||||
filter_chain.inject({}) { |h,f| h[f.filter] = f.excluded_actions; h }
|
||||
read_inheritable_attribute("excluded_actions") || {}
|
||||
end
|
||||
|
||||
# Find a filter in the filter_chain where the filter method matches the _filter_ param
|
||||
@@ -379,31 +379,22 @@ module ActionController #:nodoc:
|
||||
filter_chain.select { |f| f.filter == filter && (!block_given? || yield(f)) }.first
|
||||
end
|
||||
|
||||
# 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?
|
||||
!ia.include?(action)
|
||||
else
|
||||
(excluded_actions[filter] || []).include?(action)
|
||||
end
|
||||
end
|
||||
|
||||
# Filter class is an abstract base class for all filters. Handles all of the included/excluded actions but
|
||||
# contains no logic for calling the actual filters.
|
||||
class Filter #:nodoc:
|
||||
attr_reader :filter, :included_actions, :excluded_actions
|
||||
|
||||
def initialize(filter, options={})
|
||||
def initialize(filter)
|
||||
@filter = filter
|
||||
@position = options[:position]
|
||||
update_conditions(options)
|
||||
end
|
||||
|
||||
def update_conditions(conditions)
|
||||
if conditions[:only]
|
||||
@included_actions = [conditions[:only]].flatten.map(&:to_s)
|
||||
else
|
||||
@excluded_actions = [conditions[:except]].flatten.compact.map(&:to_s)
|
||||
end
|
||||
build_excluded_actions_hash
|
||||
end
|
||||
|
||||
def remove_actions_from_included_actions!(*actions)
|
||||
if @included_actions
|
||||
actions.flatten.map(&:to_s).each { |action| @included_actions.delete(action) }
|
||||
build_excluded_actions_hash
|
||||
end
|
||||
end
|
||||
|
||||
def before?
|
||||
@@ -414,23 +405,9 @@ module ActionController #:nodoc:
|
||||
false
|
||||
end
|
||||
|
||||
def excluded_from?(action)
|
||||
@excluded_actions_hash[action]
|
||||
end
|
||||
|
||||
def call(controller, &block)
|
||||
raise(ActionControllerError, 'No filter type: Nothing to do here.')
|
||||
end
|
||||
|
||||
private
|
||||
def build_excluded_actions_hash
|
||||
@excluded_actions_hash =
|
||||
if @included_actions
|
||||
@included_actions.inject(Hash.new(true)){|h, a| h[a] = false; h}
|
||||
else
|
||||
@excluded_actions.inject(Hash.new(false)){|h, a| h[a] = true; h}
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
# Abstract base class for filter proxies. FilterProxy objects are meant to mimic the behaviour of the old
|
||||
@@ -511,25 +488,27 @@ module ActionController #:nodoc:
|
||||
|
||||
def create_filters(filters, position, &block) #:nodoc:
|
||||
filters, conditions = extract_conditions(filters, &block)
|
||||
conditions.merge!(:position => position)
|
||||
|
||||
# conditions should be blank for a filter behind a filter proxy since the proxy will take care of all conditions.
|
||||
proxy_conditions, conditions = conditions, {} unless :around == position
|
||||
|
||||
filters.map do |filter|
|
||||
filters.map! do |filter|
|
||||
# change all the filters into instances of Filter derived classes
|
||||
class_for_filter(filter).new(filter, conditions)
|
||||
end.collect do |filter|
|
||||
class_for_filter(filter).new(filter)
|
||||
end
|
||||
|
||||
filters.map! do |filter|
|
||||
# apply proxy to filter if necessary
|
||||
case position
|
||||
when :before
|
||||
BeforeFilterProxy.new(filter, proxy_conditions)
|
||||
BeforeFilterProxy.new(filter)
|
||||
when :after
|
||||
AfterFilterProxy.new(filter, proxy_conditions)
|
||||
AfterFilterProxy.new(filter)
|
||||
else
|
||||
filter
|
||||
end
|
||||
end
|
||||
|
||||
update_conditions(filters, conditions)
|
||||
|
||||
filters
|
||||
end
|
||||
|
||||
# The determination of the filter type was once done at run time.
|
||||
@@ -553,6 +532,55 @@ module ActionController #:nodoc:
|
||||
end
|
||||
end
|
||||
|
||||
def extract_conditions(*filters, &block) #:nodoc:
|
||||
filters.flatten!
|
||||
conditions = filters.last.is_a?(Hash) ? filters.pop : {}
|
||||
filters << block if block_given?
|
||||
return filters, conditions
|
||||
end
|
||||
|
||||
def update_conditions(filters, conditions)
|
||||
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]
|
||||
end
|
||||
end
|
||||
|
||||
def condition_hash(filters, *actions)
|
||||
actions = actions.flatten.map(&:to_s)
|
||||
filters.inject({}) { |h,f| h.update( f => (actions.blank? ? nil : actions)) }
|
||||
end
|
||||
|
||||
def skip_filter_in_chain(*filters, &test) #:nodoc:
|
||||
filters, conditions = extract_conditions(filters)
|
||||
filters.map! { |f| block_given? ? find_filter(f, &test) : find_filter(f) }
|
||||
filters.compact!
|
||||
|
||||
if conditions.empty?
|
||||
delete_filters_in_chain(filters)
|
||||
else
|
||||
remove_actions_from_included_actions!(filters,conditions[:only] || [])
|
||||
conditions[:only], conditions[:except] = conditions[:except], conditions[:only]
|
||||
update_conditions(filters,conditions)
|
||||
end
|
||||
end
|
||||
|
||||
def remove_actions_from_included_actions!(filters,*actions)
|
||||
actions = actions.flatten.map(&:to_s)
|
||||
updated_hash = filters.inject(included_actions) do |hash,filter|
|
||||
ia = (hash[filter] || []) - actions
|
||||
ia.blank? ? hash.delete(filter) : hash[filter] = ia
|
||||
hash
|
||||
end
|
||||
write_inheritable_attribute('included_actions', updated_hash)
|
||||
end
|
||||
|
||||
def delete_filters_in_chain(filters) #:nodoc:
|
||||
write_inheritable_attribute('filter_chain', filter_chain.reject { |f| filters.include?(f) })
|
||||
end
|
||||
|
||||
def filter_responds_to_before_and_after(filter) #:nodoc:
|
||||
filter.respond_to?(:before) && filter.respond_to?(:after)
|
||||
end
|
||||
@@ -569,34 +597,6 @@ module ActionController #:nodoc:
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def extract_conditions(*filters, &block) #:nodoc:
|
||||
filters.flatten!
|
||||
conditions = filters.last.is_a?(Hash) ? filters.pop : {}
|
||||
filters << block if block_given?
|
||||
return filters.flatten, conditions
|
||||
end
|
||||
|
||||
def skip_filter_in_chain(*filters, &test) #:nodoc:
|
||||
filters, conditions = extract_conditions(filters)
|
||||
finder_proc = block_given? ?
|
||||
proc { |f| find_filter(f, &test) } :
|
||||
proc { |f| find_filter(f) }
|
||||
|
||||
filters.map(&finder_proc).reject(&:nil?).each do |filter|
|
||||
if conditions.empty?
|
||||
delete_filter_in_chain(filter)
|
||||
else
|
||||
filter.remove_actions_from_included_actions!(conditions[:only] || [])
|
||||
conditions[:only], conditions[:except] = conditions[:except], conditions[:only]
|
||||
filter.update_conditions(conditions)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def delete_filter_in_chain(filter) #:nodoc:
|
||||
write_inheritable_attribute('filter_chain', filter_chain.reject { |f| f == filter })
|
||||
end
|
||||
end
|
||||
|
||||
module InstanceMethods # :nodoc:
|
||||
@@ -627,7 +627,7 @@ module ActionController #:nodoc:
|
||||
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 filter.excluded_from?(action_name)
|
||||
return call_filter(chain, index.next) if self.class.filter_excluded_from_action?(filter,action_name)
|
||||
|
||||
halted = false
|
||||
filter.call(self) do
|
||||
|
||||
@@ -154,7 +154,7 @@ class FilterTest < Test::Unit::TestCase
|
||||
@ran_filter << "find_user"
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
class ConditionalParentOfConditionalSkippingController < ConditionalFilterController
|
||||
before_filter :conditional_in_parent, :only => [:show, :another_action]
|
||||
after_filter :conditional_in_parent, :only => [:show, :another_action]
|
||||
@@ -172,6 +172,10 @@ class FilterTest < Test::Unit::TestCase
|
||||
skip_after_filter :conditional_in_parent, :only => :another_action
|
||||
end
|
||||
|
||||
class AnotherChildOfConditionalParentController < ConditionalParentOfConditionalSkippingController
|
||||
skip_before_filter :conditional_in_parent, :only => :show
|
||||
end
|
||||
|
||||
class ProcController < PrependingController
|
||||
before_filter(proc { |c| c.assigns["ran_proc_filter"] = true })
|
||||
end
|
||||
@@ -413,6 +417,12 @@ class FilterTest < Test::Unit::TestCase
|
||||
assert_nil test_process(ChildOfConditionalParentController, 'another_action').template.assigns['ran_filter']
|
||||
end
|
||||
|
||||
def test_condition_skipping_of_filters_when_siblings_also_have_conditions
|
||||
assert_equal %w( conditional_in_parent conditional_in_parent ), test_process(ChildOfConditionalParentController).template.assigns['ran_filter'], "1"
|
||||
assert_equal nil, test_process(AnotherChildOfConditionalParentController).template.assigns['ran_filter']
|
||||
assert_equal %w( conditional_in_parent conditional_in_parent ), test_process(ChildOfConditionalParentController).template.assigns['ran_filter']
|
||||
end
|
||||
|
||||
private
|
||||
def test_process(controller, action = "show")
|
||||
request = ActionController::TestRequest.new
|
||||
|
||||
Reference in New Issue
Block a user