diff options
author | Ryuta Kamizono <kamipo@gmail.com> | 2019-05-03 13:36:44 +0900 |
---|---|---|
committer | Ryuta Kamizono <kamipo@gmail.com> | 2019-05-07 02:19:57 +0900 |
commit | 718a32ca745672a977a0d4ae401f61f439767405 (patch) | |
tree | 048ba67fc584728771525b5d273b5695f13f2ea4 /activerecord/lib/active_record/connection_adapters/abstract | |
parent | 020856c328085ffe4728afdab99e9008cc86d9e6 (diff) | |
download | rails-718a32ca745672a977a0d4ae401f61f439767405.tar.gz rails-718a32ca745672a977a0d4ae401f61f439767405.tar.bz2 rails-718a32ca745672a977a0d4ae401f61f439767405.zip |
Should attempt `committed!`/`rolledback!` to all enrolled records in the transaction
Currently, `committed!`/`rolledback!` will only be attempted for the
first enrolled record in the transaction, that will cause some
problematic behaviors.
The first one problem, `clear_transaction_record_state` won't be called
even if the transaction is finalized except the first enrolled record.
This means that de-duplicated records in the transaction won't refer
latest state (e.g. won't happen rolling back record state).
The second one problem, the enrolled order is not always the same as the
order in which the actions actually happened, the first enrolled record
may succeed no actions (e.g. `destroy` has already succeeded on another
record during `before_destroy`), it will lose to fire any transactional
callbacks.
To avoid both problems, we should attempt `committed!`/`rolledback!` to
all enrolled records in the transaction.
Diffstat (limited to 'activerecord/lib/active_record/connection_adapters/abstract')
-rw-r--r-- | activerecord/lib/active_record/connection_adapters/abstract/transaction.rb | 16 |
1 files changed, 12 insertions, 4 deletions
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index c9e84e48cc..7b6321d83b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -98,9 +98,13 @@ module ActiveRecord end def rollback_records - ite = records.uniq + ite = records.uniq(&:object_id) + already_run_callbacks = {} while record = ite.shift - record.rolledback!(force_restore_state: full_rollback?) + trigger_callbacks = record.trigger_transactional_callbacks? + should_run_callbacks = !already_run_callbacks[record] && trigger_callbacks + already_run_callbacks[record] ||= trigger_callbacks + record.rolledback!(force_restore_state: full_rollback?, should_run_callbacks: should_run_callbacks) end ensure ite.each do |i| @@ -113,10 +117,14 @@ module ActiveRecord end def commit_records - ite = records.uniq + ite = records.uniq(&:object_id) + already_run_callbacks = {} while record = ite.shift if @run_commit_callbacks - record.committed! + trigger_callbacks = record.trigger_transactional_callbacks? + should_run_callbacks = !already_run_callbacks[record] && trigger_callbacks + already_run_callbacks[record] ||= trigger_callbacks + record.committed!(should_run_callbacks: should_run_callbacks) else # if not running callbacks, only adds the record to the parent transaction connection.add_transaction_record(record) |