mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Use __id__ to dedup records for transactional callbacks
While not a particularly good idea, it's possible to use `object_id` as
an attribute name, typically by defining a polymorphic association named
`object`. Since 718a32ca74
, transactional
callbacks deduplicate records by their `object_id`, but this causes
incorrect behaviour when the record has an attribute with that name.
Using `__id__` instead makes a naming collision much less likely.
This commit is contained in:
parent
d5a3b2e427
commit
8d3ca97032
2 changed files with 39 additions and 2 deletions
|
@ -101,7 +101,7 @@ module ActiveRecord
|
|||
|
||||
def rollback_records
|
||||
return unless records
|
||||
ite = records.uniq(&:object_id)
|
||||
ite = records.uniq(&:__id__)
|
||||
already_run_callbacks = {}
|
||||
while record = ite.shift
|
||||
trigger_callbacks = record.trigger_transactional_callbacks?
|
||||
|
@ -121,7 +121,7 @@ module ActiveRecord
|
|||
|
||||
def commit_records
|
||||
return unless records
|
||||
ite = records.uniq(&:object_id)
|
||||
ite = records.uniq(&:__id__)
|
||||
already_run_callbacks = {}
|
||||
while record = ite.shift
|
||||
if @run_commit_callbacks
|
||||
|
|
|
@ -501,6 +501,43 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
|
|||
assert flag
|
||||
end
|
||||
|
||||
def test_saving_two_records_that_override_object_id_should_run_after_commit_callbacks_for_both
|
||||
klass = Class.new(TopicWithCallbacks) do
|
||||
define_method(:object_id) { 42 }
|
||||
end
|
||||
|
||||
records = [klass.new, klass.new]
|
||||
|
||||
klass.transaction do
|
||||
records.each do |record|
|
||||
record.after_commit_block { |r| r.history << :after_commit }
|
||||
record.save!
|
||||
end
|
||||
end
|
||||
|
||||
assert_equal [:after_commit], records.first.history
|
||||
assert_equal [:after_commit], records.second.history
|
||||
end
|
||||
|
||||
def test_saving_two_records_that_override_object_id_should_run_after_rollback_callbacks_for_both
|
||||
klass = Class.new(TopicWithCallbacks) do
|
||||
define_method(:object_id) { 42 }
|
||||
end
|
||||
|
||||
records = [klass.new, klass.new]
|
||||
|
||||
klass.transaction do
|
||||
records.each do |record|
|
||||
record.after_rollback_block { |r| r.history << :after_rollback }
|
||||
record.save!
|
||||
end
|
||||
raise ActiveRecord::Rollback
|
||||
end
|
||||
|
||||
assert_equal [:after_rollback], records.first.history
|
||||
assert_equal [:after_rollback], records.second.history
|
||||
end
|
||||
|
||||
private
|
||||
def add_transaction_execution_blocks(record)
|
||||
record.after_commit_block(:create) { |r| r.history << :commit_on_create }
|
||||
|
|
Loading…
Reference in a new issue