diff options
author | eileencodes <eileencodes@gmail.com> | 2017-07-01 14:25:33 -0400 |
---|---|---|
committer | eileencodes <eileencodes@gmail.com> | 2017-07-01 14:53:30 -0400 |
commit | 0237da287eb4c507d10a0c6d94150093acc52b03 (patch) | |
tree | 4248673d4f498716ed057a74320d08311d579e8a /activerecord/lib/active_record | |
parent | 608ebccf8f6314c945444b400a37c2d07f21b253 (diff) | |
download | rails-0237da287eb4c507d10a0c6d94150093acc52b03.tar.gz rails-0237da287eb4c507d10a0c6d94150093acc52b03.tar.bz2 rails-0237da287eb4c507d10a0c6d94150093acc52b03.zip |
Apply record state based on parent transaction state
Let's say you have a nested transaction and both records are saved.
Before the outer transaction closes, a rollback is performed. Previously
the record in the outer transaction would get marked as not persisted
but the inner transaction would get persisted.
```ruby
Post.transaction do
post_one.save # will get rolled back
Post.transaction(requires_new: true) do
post_two.save # incorrectly remains marked as persisted
end
raise ActiveRecord::Rollback
end
```
To fix this the PR changes transaction handling to have the child
transaction ask the parent how the records should be marked. When
there are child transactions, it will always be a SavpointTransaction
because the stack isn't empty. From there we pass the parent_transaction
to the child SavepointTransaction where we add the children to the parent
so the parent can mark the inner transaction as rolledback and thus mark
the record as not persisted.
`update_attributes_from_transaction_state` uses the `completed?` check to
correctly mark all the transactions as rolledback and the inner record as
not persisted.
```ruby
Post.transaction do
post_one.save # will get rolled back
Post.transaction(requires_new: true) do
post_two.save # with new behavior, correctly marked as not persisted
on rollback
end
raise ActiveRecord::Rollback
end
```
Fixes #29320
Diffstat (limited to 'activerecord/lib/active_record')
-rw-r--r-- | activerecord/lib/active_record/connection_adapters/abstract/transaction.rb | 17 | ||||
-rw-r--r-- | activerecord/lib/active_record/transactions.rb | 2 |
2 files changed, 16 insertions, 3 deletions
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index 265775c15c..2393e35ecd 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -3,6 +3,11 @@ module ActiveRecord class TransactionState def initialize(state = nil) @state = state + @children = [] + end + + def add_child(state) + @children << state end def finalized? @@ -17,6 +22,10 @@ module ActiveRecord @state == :rolledback end + def fully_completed? + completed? + end + def completed? committed? || rolledback? end @@ -40,6 +49,7 @@ module ActiveRecord end def rollback! + @children.each { |c| c.rollback! } @state = :rolledback end @@ -121,8 +131,11 @@ module ActiveRecord end class SavepointTransaction < Transaction - def initialize(connection, savepoint_name, options, *args) + def initialize(connection, savepoint_name, parent_transaction, options, *args) super(connection, options, *args) + + parent_transaction.state.add_child(@state) + if options[:isolation] raise ActiveRecord::TransactionIsolationError, "cannot set transaction isolation in a nested transaction" end @@ -176,7 +189,7 @@ module ActiveRecord if @stack.empty? RealTransaction.new(@connection, options, run_commit_callbacks: run_commit_callbacks) else - SavepointTransaction.new(@connection, "active_record_#{@stack.size}", options, + SavepointTransaction.new(@connection, "active_record_#{@stack.size}", @stack.last, options, run_commit_callbacks: run_commit_callbacks) end diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 45795fa287..463bb1f314 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -490,7 +490,7 @@ module ActiveRecord def update_attributes_from_transaction_state(transaction_state) if transaction_state && transaction_state.finalized? restore_transaction_record_state if transaction_state.rolledback? - clear_transaction_record_state + clear_transaction_record_state if transaction_state.fully_completed? end end end |