diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index b71f3a51f3e415d420362d43af7ac248f3d49765..33c72f743df5d3c9ab7fafb743bbc2c628185892 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
*SVN*
+* Refactor filters to use Active Support callbacks. #11235 [Josh Peek]
+
* Fixed that polymorphic routes would modify the input array #11363 [thomas.lee]
* Added :format option to NumberHelper#number_to_currency to enable better localization support #11149 [lylo]
diff --git a/actionpack/lib/action_controller/dispatcher.rb b/actionpack/lib/action_controller/dispatcher.rb
index f892058f2095f71ddd0cbdfee40f583a4e6b2ddc..92576bdb2bced1749f7217f73a79c40f37769b8d 100644
--- a/actionpack/lib/action_controller/dispatcher.rb
+++ b/actionpack/lib/action_controller/dispatcher.rb
@@ -20,17 +20,9 @@ def dispatch(cgi = nil, session_options = CgiRequest::DEFAULT_SESSION_OPTIONS, o
# existing callback. Passing an identifier is a suggested practice if the
# code adding a preparation block may be reloaded.
def to_prepare(identifier = nil, &block)
- @prepare_dispatch_callbacks ||= []
+ @prepare_dispatch_callbacks ||= ActiveSupport::Callbacks::CallbackChain.new
callback = ActiveSupport::Callbacks::Callback.new(:prepare_dispatch, block, :identifier => identifier)
-
- # Already registered: update the existing callback
- # TODO: Ruby one liner for Array#find returning index
- if identifier && callback_for_identifier = @prepare_dispatch_callbacks.find { |c| c.identifier == identifier }
- index = @prepare_dispatch_callbacks.index(callback_for_identifier)
- @prepare_dispatch_callbacks[index] = callback
- else
- @prepare_dispatch_callbacks.concat([callback])
- end
+ @prepare_dispatch_callbacks.replace_or_append_callback(callback)
end
# If the block raises, send status code as a last-ditch response.
diff --git a/actionpack/lib/action_controller/filters.rb b/actionpack/lib/action_controller/filters.rb
index 337c489f07e8b4f9a1345b8c57ec80b6b3764647..a822ffe4e4f4348dcf762d1e0d16d49dc5ab42c8 100644
--- a/actionpack/lib/action_controller/filters.rb
+++ b/actionpack/lib/action_controller/filters.rb
@@ -244,17 +244,203 @@ def self.included(base)
# filter and controller action will not be run. If #before renders or redirects,
# 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 filters will be appended to the filter_chain and
# will execute before the action on this controller is performed.
def append_before_filter(*filters, &block)
- append_filter_to_chain(filters, :before, &block)
+ filter_chain.append_filter_to_chain(filters, :before, &block)
end
# The passed filters will be prepended to the filter_chain and
# will execute before the action on this controller is performed.
def prepend_before_filter(*filters, &block)
- prepend_filter_to_chain(filters, :before, &block)
+ filter_chain.prepend_filter_to_chain(filters, :before, &block)
end
# Shorthand for append_before_filter since it's the most common.
@@ -263,19 +449,18 @@ def prepend_before_filter(*filters, &block)
# The passed filters will be appended to the array of filters
# that run _after_ actions on this controller are performed.
def append_after_filter(*filters, &block)
- append_filter_to_chain(filters, :after, &block)
+ filter_chain.append_filter_to_chain(filters, :after, &block)
end
# The passed filters will be prepended to the array of filters
# that run _after_ actions on this controller are performed.
def prepend_after_filter(*filters, &block)
- prepend_filter_to_chain(filters, :after, &block)
+ filter_chain.prepend_filter_to_chain(filters, :after, &block)
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
#
# B#before
@@ -287,10 +472,7 @@ def prepend_after_filter(*filters, &block)
# With around filters which yield to the action block, #before and #after
# are the code before and after the yield.
def append_around_filter(*filters, &block)
- filters, conditions = extract_conditions(filters, &block)
- filters.map { |f| proxy_before_and_after_filter(f) }.each do |filter|
- append_filter_to_chain([filter, conditions])
- end
+ filter_chain.append_filter_to_chain(filters, :around, &block)
end
# If you prepend_around_filter A.new, B.new, the filter chain looks like:
@@ -304,10 +486,7 @@ def append_around_filter(*filters, &block)
# With around filters which yield to the action block, #before and #after
# are the code before and after the yield.
def prepend_around_filter(*filters, &block)
- filters, conditions = extract_conditions(filters, &block)
- filters.map { |f| proxy_before_and_after_filter(f) }.each do |filter|
- prepend_filter_to_chain([filter, conditions])
- end
+ filter_chain.prepend_filter_to_chain(filters, :around, &block)
end
# Shorthand for append_around_filter since it's the most common.
@@ -320,7 +499,7 @@ def prepend_around_filter(*filters, &block)
# You can control the actions to skip the filter for with the :only and :except options,
# just like when you apply the filters.
def skip_before_filter(*filters)
- skip_filter_in_chain(*filters, &:before?)
+ filter_chain.skip_filter_in_chain(*filters, &:before?)
end
# Removes the specified filters from the +after+ filter chain. Note that this only works for skipping method-reference
@@ -330,7 +509,7 @@ def skip_before_filter(*filters)
# You can control the actions to skip the filter for with the :only and :except options,
# just like when you apply the filters.
def skip_after_filter(*filters)
- skip_filter_in_chain(*filters, &:after?)
+ filter_chain.skip_filter_in_chain(*filters, &:after?)
end
# Removes the specified filters from the filter chain. This only works for method reference (symbol)
@@ -340,336 +519,30 @@ def skip_after_filter(*filters)
# You can control the actions to skip the filter for with the :only and :except options,
# just like when you apply the filters.
def skip_filter(*filters)
- skip_filter_in_chain(*filters)
+ filter_chain.skip_filter_in_chain(*filters)
end
# Returns an array of Filter objects for this controller.
def filter_chain
- read_inheritable_attribute("filter_chain") || []
+ if chain = read_inheritable_attribute('filter_chain')
+ return chain
+ else
+ write_inheritable_attribute('filter_chain', FilterChain.new)
+ return filter_chain
+ end
end
# Returns all the before filters for this class and all its ancestors.
# This method returns the actual filter that was assigned in the controller to maintain existing functionality.
def before_filters #:nodoc:
- filter_chain.select(&:before?).map(&:filter)
+ filter_chain.select(&:before?).map(&:method)
end
# Returns all the after filters for this class and all its ancestors.
# This method returns the actual filter that was assigned in the controller to maintain existing functionality.
def after_filters #:nodoc:
- filter_chain.select(&:after?).map(&:filter)
- end
-
- # Returns a mapping between filters and the actions that may run them.
- def included_actions #:nodoc:
- @included_actions ||= read_inheritable_attribute("included_actions") || {}
- end
-
- # Returns a mapping between filters and actions that may not run them.
- def excluded_actions #:nodoc:
- @excluded_actions ||= read_inheritable_attribute("excluded_actions") || {}
- end
-
- # Find a filter in the filter_chain where the filter method matches the _filter_ param
- # and (optionally) the passed block evaluates to true (mostly used for testing before?
- # and after? on the filter). Useful for symbol filters.
- #
- # The object of type Filter is passed to the block when yielded, not the filter itself.
- def find_filter(filter, &block) #:nodoc:
- filter_chain.select { |f| f.filter == filter && (!block_given? || yield(f)) }.first
+ filter_chain.select(&:after?).map(&:method)
end
-
- # Returns true if the filter is excluded from the given action
- def filter_excluded_from_action?(filter,action) #:nodoc:
- case
- when ia = included_actions[filter]
- !ia.include?(action)
- when ea = excluded_actions[filter]
- ea.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)
- @filter = filter
- end
-
- def type
- :around
- end
-
- def before?
- type == :before
- end
-
- def after?
- type == :after
- end
-
- def around?
- type == :around
- end
-
- def run(controller)
- raise ActionControllerError, 'No filter type: Nothing to do here.'
- end
-
- def call(controller, &block)
- run(controller)
- end
- end
-
- # Abstract base class for filter proxies. FilterProxy objects are meant to mimic the behaviour of the old
- # before_filter and after_filter by moving the logic into the filter itself.
- class FilterProxy < Filter #:nodoc:
- def filter
- @filter.filter
- end
- end
-
- class BeforeFilterProxy < FilterProxy #:nodoc:
- def type
- :before
- end
-
- def run(controller)
- # only filters returning false are halted.
- @filter.call(controller)
- if controller.send!(:performed?)
- controller.send!(:halt_filter_chain, @filter, :rendered_or_redirected)
- end
- end
-
- def call(controller)
- yield unless run(controller)
- end
- end
-
- class AfterFilterProxy < FilterProxy #:nodoc:
- def type
- :after
- end
-
- def run(controller)
- @filter.call(controller)
- end
-
- def call(controller)
- yield
- run(controller)
- end
- end
-
- class SymbolFilter < Filter #:nodoc:
- def call(controller, &block)
- controller.send!(@filter, &block)
- end
- end
-
- class ProcFilter < Filter #:nodoc:
- def call(controller)
- @filter.call(controller)
- rescue LocalJumpError # a yield from a proc... no no bad dog.
- raise(ActionControllerError, 'Cannot yield from a Proc type filter. The Proc must take two arguments and execute #call on the second argument.')
- end
- end
-
- class ProcWithCallFilter < Filter #:nodoc:
- def call(controller, &block)
- @filter.call(controller, block)
- rescue LocalJumpError # a yield from a proc... no no bad dog.
- raise(ActionControllerError, 'Cannot yield from a Proc type filter. The Proc must take two arguments and execute #call on the second argument.')
- end
- end
-
- class MethodFilter < Filter #:nodoc:
- def call(controller, &block)
- @filter.call(controller, &block)
- end
- end
-
- class ClassFilter < Filter #:nodoc:
- def call(controller, &block)
- @filter.filter(controller, &block)
- 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, 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 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
- 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)
- # 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
-
- def create_filters(filters, filter_type, &block) #:nodoc:
- filters, conditions = extract_conditions(filters, &block)
- filters.map! { |filter| find_or_create_filter(filter, filter_type) }
- update_conditions(filters, conditions)
- filters
- end
-
- 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, filter_type).new(filter)
- # apply proxy to filter if necessary
- case filter_type
- when :before
- BeforeFilterProxy.new(f)
- when :after
- AfterFilterProxy.new(f)
- else
- f
- end
- end
- end
-
- # 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, filter_type) #:nodoc:
- case
- when filter.is_a?(Symbol)
- SymbolFilter
- when filter.respond_to?(:call)
- if filter.is_a?(Method)
- MethodFilter
- else
- case filter.arity
- when 1; ProcFilter
- when 2; ProcWithCallFilter
- else raise ArgumentError, 'Filter blocks must take one or two arguments.'
- end
- 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 filter must be a Symbol, Proc, Method, or object responding to filter, after or before.')
- end
- end
-
- def extract_conditions(*filters, &block) #:nodoc:
- filters.flatten!
- conditions = filters.extract_options!
- 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]))
- elsif conditions[:except]
- write_inheritable_hash('excluded_actions', condition_hash(filters, 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(read_inheritable_attribute('included_actions')||{}) do |hash,filter|
- ia = (hash[filter] || []) - actions
- ia.empty? ? 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
-
- def proxy_before_and_after_filter(filter) #:nodoc:
- return filter unless filter_responds_to_before_and_after(filter)
- Proc.new do |controller, action|
- filter.before(controller)
-
- if controller.send!(:performed?)
- controller.send!(:halt_filter_chain, filter, :rendered_or_redirected)
- else
- begin
- action.call
- ensure
- filter.after(controller)
- end
- end
- end
- end
end
module InstanceMethods # :nodoc:
@@ -681,89 +554,80 @@ def self.included(base)
end
protected
+ def process_with_filters(request, response, method = :perform_action, *arguments) #:nodoc:
+ @before_filter_chain_aborted = false
+ process_without_filters(request, response, method, *arguments)
+ end
- def process_with_filters(request, response, method = :perform_action, *arguments) #:nodoc:
- @before_filter_chain_aborted = false
- process_without_filters(request, response, method, *arguments)
- end
-
- def perform_action_with_filters
- call_filters(self.class.filter_chain, 0, 0)
- end
+ def perform_action_with_filters
+ call_filters(self.class.filter_chain, 0, 0)
+ end
private
+ def call_filters(chain, index, nesting)
+ index = run_before_filters(chain, index, nesting)
+ aborted = @before_filter_chain_aborted
+ perform_action_without_filters unless performed? || aborted
+ return index if nesting != 0 || aborted
+ run_after_filters(chain, index)
+ end
+
+ def run_before_filters(chain, index, nesting)
+ while chain[index]
+ filter, index = chain[index], index
+ break unless filter # end of call chain reached
+
+ case filter
+ when BeforeFilter
+ filter.call(self) # invoke before filter
+ index = index.next
+ break if @before_filter_chain_aborted
+ when AroundFilter
+ yielded = false
+
+ filter.call(self) do
+ yielded = true
+ # all remaining before and around filters will be run in this call
+ index = call_filters(chain, index.next, nesting.next)
+ end
- def call_filters(chain, index, nesting)
- index = run_before_filters(chain, index, nesting)
- aborted = @before_filter_chain_aborted
- perform_action_without_filters unless performed? || aborted
- return index if nesting != 0 || aborted
- run_after_filters(chain, index)
- end
-
- def skip_excluded_filters(chain, index)
- while (filter = chain[index]) && self.class.filter_excluded_from_action?(filter, action_name)
- index = index.next
- end
- [filter, index]
- end
-
- def run_before_filters(chain, index, nesting)
- while chain[index]
- filter, index = skip_excluded_filters(chain, index)
- break unless filter # end of call chain reached
+ halt_filter_chain(filter, :did_not_yield) unless yielded
- case filter.type
- when :before
- filter.run(self) # invoke before filter
- index = index.next
- break if @before_filter_chain_aborted
- when :around
- yielded = false
-
- filter.call(self) do
- yielded = true
- # all remaining before and around filters will be run in this call
- index = call_filters(chain, index.next, nesting.next)
+ break
+ else
+ break # no before or around filters left
end
-
- halt_filter_chain(filter, :did_not_yield) unless yielded
-
- break
- else
- break # no before or around filters left
end
+
+ index
end
- index
- end
+ def run_after_filters(chain, index)
+ seen_after_filter = false
- def run_after_filters(chain, index)
- seen_after_filter = false
+ while chain[index]
+ filter, index = chain[index], index
+ break unless filter # end of call chain reached
- while chain[index]
- filter, index = skip_excluded_filters(chain, index)
- break unless filter # end of call chain reached
+ case filter
+ when AfterFilter
+ seen_after_filter = true
+ filter.call(self) # invoke after filter
+ else
+ # implementation error or someone has mucked with the filter chain
+ raise ActionControllerError, "filter #{filter.inspect} was in the wrong place!" if seen_after_filter
+ end
- case filter.type
- when :after
- seen_after_filter = true
- filter.run(self) # invoke after filter
- else
- # implementation error or someone has mucked with the filter chain
- raise ActionControllerError, "filter #{filter.inspect} was in the wrong place!" if seen_after_filter
+ index = index.next
end
- index = index.next
+ index.next
end
- index.next
- end
-
- def halt_filter_chain(filter, reason)
- @before_filter_chain_aborted = true
- logger.info "Filter chain halted as [#{filter.inspect}] #{reason}." if logger
- end
+ def halt_filter_chain(filter, reason)
+ @before_filter_chain_aborted = true
+ logger.info "Filter chain halted as [#{filter.inspect}] #{reason}." if logger
+ end
end
end
end
diff --git a/actionpack/test/controller/dispatcher_test.rb b/actionpack/test/controller/dispatcher_test.rb
index fe9789c69824265fdb50219ad3538bd4d0300f93..c64defeed4df972f627365d2235691a69496bec6 100644
--- a/actionpack/test/controller/dispatcher_test.rb
+++ b/actionpack/test/controller/dispatcher_test.rb
@@ -11,7 +11,7 @@ def setup
@output = StringIO.new
ENV['REQUEST_METHOD'] = 'GET'
- Dispatcher.instance_variable_set("@prepare_dispatch_callbacks", [])
+ Dispatcher.instance_variable_set("@prepare_dispatch_callbacks", ActiveSupport::Callbacks::CallbackChain.new)
@dispatcher = Dispatcher.new(@output)
end
diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb
index 7cf2c9d318123194d6e29827a4ebed59ceaf2e8f..092eb45cabc4e0b56aa972b811dd9144bee9411c 100644
--- a/actionpack/test/controller/filters_test.rb
+++ b/actionpack/test/controller/filters_test.rb
@@ -134,6 +134,11 @@ class AnomolousYetValidConditionController < ConditionalFilterController
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 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
def show
@@ -466,6 +471,11 @@ def test_running_anomolous_yet_valid_condition_filters
assert !response.template.assigns["ran_proc_filter2"]
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"]
assert_equal nil, test_process(ConditionalCollectionFilterController, "show_without_filter").template.assigns["ran_filter"]
@@ -499,13 +509,6 @@ def test_running_before_and_after_condition_filters
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
-
def test_around_filter
controller = test_process(AroundFilterController)
assert controller.template.assigns["before_ran"]
@@ -746,14 +749,6 @@ def test_filters_registering
assert_equal 4, ControllerWithAllTypesOfFilters.filter_chain.size
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
assert_nothing_raised { test_process(controller,'no_raise') }
diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb
index f9888c71118ae35f1cdf4aaba9252e74c36b4606..7775cf93c5896a952ba0ab1cc12638516301a03d 100755
--- a/activerecord/lib/active_record/validations.rb
+++ b/activerecord/lib/active_record/validations.rb
@@ -282,19 +282,20 @@ def self.included(base) # :nodoc:
base.send :include, ActiveSupport::Callbacks
- # TODO: Use helper ActiveSupport::Callbacks#define_callbacks instead
- %w( validate validate_on_create validate_on_update ).each do |validation_method|
+ VALIDATIONS.each do |validation_method|
base.class_eval <<-"end_eval"
def self.#{validation_method}(*methods, &block)
- options = methods.extract_options!
- methods << block if block_given?
- methods.map! { |method| Callback.new(:#{validation_method}, method, options) }
- existing_methods = read_inheritable_attribute(:#{validation_method}) || []
- write_inheritable_attribute(:#{validation_method}, existing_methods | methods)
+ methods = CallbackChain.build(:#{validation_method}, *methods, &block)
+ self.#{validation_method}_callback_chain.replace(#{validation_method}_callback_chain | methods)
end
def self.#{validation_method}_callback_chain
- read_inheritable_attribute(:#{validation_method}) || []
+ if chain = read_inheritable_attribute(:#{validation_method})
+ return chain
+ else
+ write_inheritable_attribute(:#{validation_method}, CallbackChain.new)
+ return #{validation_method}_callback_chain
+ end
end
end_eval
end
diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb
index 9831e82319113a97e6b3ae823209a96fd79aa3a8..40d71af69d31d012dcedea2fece815ee3dc8fb9a 100644
--- a/activesupport/lib/active_support/callbacks.rb
+++ b/activesupport/lib/active_support/callbacks.rb
@@ -76,20 +76,53 @@ module ActiveSupport
# - save
# saved
module Callbacks
- class Callback
- def self.run(callbacks, object, options = {}, &terminator)
- enumerator = options[:enumerator] || :each
+ class CallbackChain < Array
+ def self.build(kind, *methods, &block)
+ methods, options = extract_options(*methods, &block)
+ methods.map! { |method| Callback.new(kind, method, options) }
+ new(methods)
+ end
+
+ def run(object, options = {}, &terminator)
+ enumerator = options[:enumerator] || :each
unless block_given?
- callbacks.send(enumerator) { |callback| callback.call(object) }
+ send(enumerator) { |callback| callback.call(object) }
else
- callbacks.send(enumerator) do |callback|
+ send(enumerator) do |callback|
result = callback.call(object)
break result if terminator.call(result, object)
end
end
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
def initialize(kind, method, options = {})
@@ -99,22 +132,50 @@ def initialize(kind, method, options = {})
@options = options
end
- def call(object)
- evaluate_method(method, object) if should_run_callback?(object)
+ def ==(other)
+ case other
+ when Callback
+ (self.identifier && self.identifier == other.identifier) || self.method == other.method
+ else
+ (self.identifier && self.identifier == other) || self.method == other
+ end
+ end
+
+ def eql?(other)
+ self == other
+ end
+
+ def dup
+ self.class.new(@kind, @method, @options.dup)
+ end
+
+ def call(object, &block)
+ evaluate_method(method, object, &block) if should_run_callback?(object)
+ rescue LocalJumpError
+ raise ArgumentError,
+ "Cannot yield from a Proc type filter. The Proc must take two " +
+ "arguments and execute #call on the second argument."
end
private
- def evaluate_method(method, object)
+ def evaluate_method(method, object, &block)
case method
when Symbol
- object.send(method)
+ object.send(method, &block)
when String
eval(method, object.instance_eval { binding })
when Proc, Method
- method.call(object)
+ case method.arity
+ when -1, 1
+ method.call(object, &block)
+ when 2
+ method.call(object, block)
+ else
+ raise ArgumentError, 'Callback blocks must take one or two arguments.'
+ end
else
if method.respond_to?(kind)
- method.send(kind, object)
+ method.send(kind, object, &block)
else
raise ArgumentError,
"Callbacks must be a symbol denoting the method to call, a string to be evaluated, " +
@@ -143,17 +204,15 @@ def define_callbacks(*callbacks)
callbacks.each do |callback|
class_eval <<-"end_eval"
def self.#{callback}(*methods, &block)
- options = methods.extract_options!
- methods << block if block_given?
- callbacks = methods.map { |method| Callback.new(:#{callback}, method, options) }
- (@#{callback}_callbacks ||= []).concat callbacks
+ callbacks = CallbackChain.build(:#{callback}, *methods, &block)
+ (@#{callback}_callbacks ||= CallbackChain.new).concat callbacks
end
def self.#{callback}_callback_chain
- @#{callback}_callbacks ||= []
+ @#{callback}_callbacks ||= CallbackChain.new
if superclass.respond_to?(:#{callback}_callback_chain)
- superclass.#{callback}_callback_chain + @#{callback}_callbacks
+ CallbackChain.new(superclass.#{callback}_callback_chain + @#{callback}_callbacks)
else
@#{callback}_callbacks
end
@@ -208,7 +267,7 @@ def self.#{callback}_callback_chain
# pass
# stop
def run_callbacks(kind, options = {}, &block)
- Callback.run(self.class.send("#{kind}_callback_chain"), self, options, &block)
+ self.class.send("#{kind}_callback_chain").run(self, options, &block)
end
end
end
diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb
index 6d390bbc5cd2258c32c2c28fa43c3ac11e8a8cf5..3f8cb7f01a974dc100f217ab4e3db0d25730528f 100644
--- a/activesupport/test/callbacks_test.rb
+++ b/activesupport/test/callbacks_test.rb
@@ -94,3 +94,24 @@ def test_save_conditional_person
], person.history
end
end
+
+class CallbackTest < Test::Unit::TestCase
+ def test_eql
+ callback = Callback.new(:before, :save, :identifier => :lifesaver)
+ assert callback.eql?(Callback.new(:before, :save, :identifier => :lifesaver))
+ assert callback.eql?(Callback.new(:before, :save))
+ assert callback.eql?(:lifesaver)
+ assert callback.eql?(:save)
+ assert !callback.eql?(Callback.new(:before, :destroy))
+ assert !callback.eql?(:destroy)
+ end
+
+ def test_dup
+ a = Callback.new(:before, :save)
+ assert_equal({}, a.options)
+ b = a.dup
+ b.options[:unless] = :pigs_fly
+ assert_equal({:unless => :pigs_fly}, b.options)
+ assert_equal({}, a.options)
+ end
+end