Fixes skipping object callback filters
This allows you to skip callbacks that are defined by objects, e.g. for `ActionController`: skip_after_filter MySpecialFilter Previously this didn't work due to a bug in how Rails compared callbacks in `Callback#matches?`. When a callback is compiled, if it's an object filter (i.e. not a method, proc, etc.), `Callback` now defines a method on `@klass` that is derived from the class name rather than `@callback_id`. So, when `skip_callback` tries to find the appropriate callback to remove, `Callback` can regenerate the method name for the filter object and return the correct value for `Callback#matches?`.
This commit is contained in:
parent
e456ad514a
commit
8e1d3cd490
@ -14,6 +14,9 @@
|
||||
|
||||
*Charles Jones*
|
||||
|
||||
* Fix skipping of filters defined by objects in `ActiveSupport::Callbacks::Callback`.
|
||||
|
||||
*Ben McRedmond*
|
||||
|
||||
## Rails 4.0.0.beta1 (February 25, 2013) ##
|
||||
|
||||
|
@ -131,7 +131,15 @@ def next_id
|
||||
end
|
||||
|
||||
def matches?(_kind, _filter)
|
||||
@kind == _kind && @filter == _filter
|
||||
_filter_matches = false
|
||||
|
||||
if @_is_object_filter
|
||||
_filter_matches = @filter.to_s.start_with?(_method_name_for_object_filter(_kind, _filter, false))
|
||||
else
|
||||
_filter_matches = (@filter == _filter)
|
||||
end
|
||||
|
||||
@kind == _kind && _filter_matches
|
||||
end
|
||||
|
||||
def duplicates?(other)
|
||||
@ -234,6 +242,16 @@ def recompile_options!
|
||||
@compiled_options = conditions.flatten.join(" && ")
|
||||
end
|
||||
|
||||
def _method_name_for_object_filter(kind, filter, append_next_id = true)
|
||||
class_name = filter.kind_of?(Class) ? filter.to_s : filter.class.to_s
|
||||
class_name.gsub!(/<|>|#/, '')
|
||||
class_name.gsub!(/\/|:/, "_")
|
||||
|
||||
method_name = "_callback_#{kind}_#{class_name}"
|
||||
method_name << "_#{next_id}" if append_next_id
|
||||
method_name
|
||||
end
|
||||
|
||||
# Filters support:
|
||||
#
|
||||
# Arrays:: Used in conditions. This is used to specify
|
||||
@ -255,6 +273,8 @@ def recompile_options!
|
||||
# a method is created that calls the before_foo method
|
||||
# on the object.
|
||||
def _compile_filter(filter)
|
||||
@_is_object_filter = false
|
||||
|
||||
case filter
|
||||
when Array
|
||||
filter.map {|f| _compile_filter(f)}
|
||||
@ -269,7 +289,8 @@ def _compile_filter(filter)
|
||||
|
||||
method_name << (filter.arity == 1 ? "(self)" : " self, Proc.new ")
|
||||
else
|
||||
method_name = "_callback_#{@kind}_#{next_id}"
|
||||
method_name = _method_name_for_object_filter(kind, filter)
|
||||
@_is_object_filter = true
|
||||
@klass.send(:define_method, "#{method_name}_object") { filter }
|
||||
|
||||
_normalize_legacy_filter(kind, filter)
|
||||
|
@ -66,6 +66,16 @@ def history
|
||||
end
|
||||
end
|
||||
|
||||
class CallbackClass
|
||||
def self.before(model)
|
||||
model.history << [:before_save, :class]
|
||||
end
|
||||
|
||||
def self.after(model)
|
||||
model.history << [:after_save, :class]
|
||||
end
|
||||
end
|
||||
|
||||
class Person < Record
|
||||
[:before_save, :after_save].each do |callback_method|
|
||||
callback_method_sym = callback_method.to_sym
|
||||
@ -73,6 +83,7 @@ class Person < Record
|
||||
send(callback_method, callback_string(callback_method_sym))
|
||||
send(callback_method, callback_proc(callback_method_sym))
|
||||
send(callback_method, callback_object(callback_method_sym.to_s.gsub(/_save/, '')))
|
||||
send(callback_method, CallbackClass)
|
||||
send(callback_method) { |model| model.history << [callback_method_sym, :block] }
|
||||
end
|
||||
|
||||
@ -86,6 +97,7 @@ class PersonSkipper < Person
|
||||
skip_callback :save, :after, :before_save_method, :unless => :yes
|
||||
skip_callback :save, :after, :before_save_method, :if => :no
|
||||
skip_callback :save, :before, :before_save_method, :unless => :no
|
||||
skip_callback :save, :before, CallbackClass , :if => :yes
|
||||
def yes; true; end
|
||||
def no; false; end
|
||||
end
|
||||
@ -430,6 +442,7 @@ def test_skip_person
|
||||
[:before_save, :object],
|
||||
[:before_save, :block],
|
||||
[:after_save, :block],
|
||||
[:after_save, :class],
|
||||
[:after_save, :object],
|
||||
[:after_save, :proc],
|
||||
[:after_save, :string],
|
||||
@ -449,8 +462,10 @@ def test_save_person
|
||||
[:before_save, :string],
|
||||
[:before_save, :proc],
|
||||
[:before_save, :object],
|
||||
[:before_save, :class],
|
||||
[:before_save, :block],
|
||||
[:after_save, :block],
|
||||
[:after_save, :class],
|
||||
[:after_save, :object],
|
||||
[:after_save, :proc],
|
||||
[:after_save, :string],
|
||||
@ -715,8 +730,10 @@ def test_skip_writer
|
||||
[:before_save, :string],
|
||||
[:before_save, :proc],
|
||||
[:before_save, :object],
|
||||
[:before_save, :class],
|
||||
[:before_save, :block],
|
||||
[:after_save, :block],
|
||||
[:after_save, :class],
|
||||
[:after_save, :object],
|
||||
[:after_save, :proc],
|
||||
[:after_save, :string],
|
||||
|
Loading…
Reference in New Issue
Block a user