Merge pull request #7376 from dmitriy-kiriyenko/fix-double-callback-in-same-statement
Prevent callback from being set twice. Conflicts: activesupport/CHANGELOG.md
This commit is contained in:
commit
ecc7751575
@ -1,5 +1,14 @@
|
||||
## Rails 4.0.0 (unreleased) ##
|
||||
|
||||
* Prevent `Callbacks#set_callback` from setting the same callback twice.
|
||||
|
||||
before_save :foo, :bar, :foo
|
||||
|
||||
will at first call `bar`, then `foo`. `foo` will no more be called
|
||||
twice.
|
||||
|
||||
*Dmitriy Kiriyenko*
|
||||
|
||||
* Add ActiveSupport::Logger#silence that works the same as the old Logger#silence extension.
|
||||
|
||||
*DHH*
|
||||
|
@ -134,6 +134,10 @@ def matches?(_kind, _filter)
|
||||
@kind == _kind && @filter == _filter
|
||||
end
|
||||
|
||||
def duplicates?(other)
|
||||
matches?(other.kind, other.filter)
|
||||
end
|
||||
|
||||
def _update_filter(filter_options, new_options)
|
||||
filter_options[:if].concat(Array(new_options[:unless])) if new_options.key?(:unless)
|
||||
filter_options[:unless].concat(Array(new_options[:if])) if new_options.key?(:if)
|
||||
@ -328,6 +332,30 @@ def compile
|
||||
method.join("\n")
|
||||
end
|
||||
|
||||
def append(*callbacks)
|
||||
callbacks.each { |c| append_one(c) }
|
||||
end
|
||||
|
||||
def prepend(*callbacks)
|
||||
callbacks.each { |c| prepend_one(c) }
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def append_one(callback)
|
||||
remove_duplicates(callback)
|
||||
push(callback)
|
||||
end
|
||||
|
||||
def prepend_one(callback)
|
||||
remove_duplicates(callback)
|
||||
unshift(callback)
|
||||
end
|
||||
|
||||
def remove_duplicates(callback)
|
||||
delete_if { |c| callback.duplicates?(c) }
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
module ClassMethods
|
||||
@ -421,11 +449,7 @@ def set_callback(name, *filter_list, &block)
|
||||
Callback.new(chain, filter, type, options.dup, self)
|
||||
end
|
||||
|
||||
filters.each do |filter|
|
||||
chain.delete_if {|c| c.matches?(type, filter) }
|
||||
end
|
||||
|
||||
options[:prepend] ? chain.unshift(*(mapped.reverse)) : chain.push(*mapped)
|
||||
options[:prepend] ? chain.prepend(*mapped) : chain.append(*mapped)
|
||||
|
||||
target.send("_#{name}_callbacks=", chain)
|
||||
end
|
||||
|
@ -607,6 +607,45 @@ def save
|
||||
end
|
||||
end
|
||||
|
||||
class OneTwoThreeSave
|
||||
include ActiveSupport::Callbacks
|
||||
|
||||
define_callbacks :save
|
||||
|
||||
attr_accessor :record
|
||||
|
||||
def initialize
|
||||
@record = []
|
||||
end
|
||||
|
||||
def save
|
||||
run_callbacks :save do
|
||||
@record << "yielded"
|
||||
end
|
||||
end
|
||||
|
||||
def first
|
||||
@record << "one"
|
||||
end
|
||||
|
||||
def second
|
||||
@record << "two"
|
||||
end
|
||||
|
||||
def third
|
||||
@record << "three"
|
||||
end
|
||||
end
|
||||
|
||||
class DuplicatingCallbacks < OneTwoThreeSave
|
||||
set_callback :save, :before, :first, :second
|
||||
set_callback :save, :before, :first, :third
|
||||
end
|
||||
|
||||
class DuplicatingCallbacksInSameCall < OneTwoThreeSave
|
||||
set_callback :save, :before, :first, :second, :first, :third
|
||||
end
|
||||
|
||||
class UsingObjectTest < ActiveSupport::TestCase
|
||||
def test_before_object
|
||||
u = UsingObjectBefore.new
|
||||
@ -710,4 +749,17 @@ def test_per_key_option_deprecaton
|
||||
end
|
||||
end
|
||||
|
||||
class ExcludingDuplicatesCallbackTest < ActiveSupport::TestCase
|
||||
def test_excludes_duplicates_in_separate_calls
|
||||
model = DuplicatingCallbacks.new
|
||||
model.save
|
||||
assert_equal ["two", "one", "three", "yielded"], model.record
|
||||
end
|
||||
|
||||
def test_excludes_duplicates_in_one_call
|
||||
model = DuplicatingCallbacksInSameCall.new
|
||||
model.save
|
||||
assert_equal ["two", "one", "three", "yielded"], model.record
|
||||
end
|
||||
end
|
||||
end
|
||||
|
@ -122,12 +122,12 @@ class AlsoDoingNothingTest < ActiveSupport::TestCase
|
||||
# Setup and teardown callbacks.
|
||||
class SetupAndTeardownTest < ActiveSupport::TestCase
|
||||
setup :reset_callback_record, :foo
|
||||
teardown :foo, :sentinel, :foo
|
||||
teardown :foo, :sentinel
|
||||
|
||||
def test_inherited_setup_callbacks
|
||||
assert_equal [:reset_callback_record, :foo], self.class._setup_callbacks.map(&:raw_filter)
|
||||
assert_equal [:foo], @called_back
|
||||
assert_equal [:foo, :sentinel, :foo], self.class._teardown_callbacks.map(&:raw_filter)
|
||||
assert_equal [:foo, :sentinel], self.class._teardown_callbacks.map(&:raw_filter)
|
||||
end
|
||||
|
||||
def setup
|
||||
@ -147,7 +147,7 @@ def foo
|
||||
end
|
||||
|
||||
def sentinel
|
||||
assert_equal [:foo, :foo], @called_back
|
||||
assert_equal [:foo], @called_back
|
||||
end
|
||||
end
|
||||
|
||||
@ -159,7 +159,7 @@ class SubclassSetupAndTeardownTest < SetupAndTeardownTest
|
||||
def test_inherited_setup_callbacks
|
||||
assert_equal [:reset_callback_record, :foo, :bar], self.class._setup_callbacks.map(&:raw_filter)
|
||||
assert_equal [:foo, :bar], @called_back
|
||||
assert_equal [:foo, :sentinel, :foo, :bar], self.class._teardown_callbacks.map(&:raw_filter)
|
||||
assert_equal [:foo, :sentinel, :bar], self.class._teardown_callbacks.map(&:raw_filter)
|
||||
end
|
||||
|
||||
protected
|
||||
@ -168,7 +168,7 @@ def bar
|
||||
end
|
||||
|
||||
def sentinel
|
||||
assert_equal [:foo, :bar, :bar, :foo], @called_back
|
||||
assert_equal [:foo, :bar, :bar], @called_back
|
||||
end
|
||||
end
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user