From 998ba0e7084345a045d49a64ee9cf9d2d2441780 Mon Sep 17 00:00:00 2001 From: Bogdan Gusiev Date: Wed, 4 Dec 2013 16:16:11 +0200 Subject: Fix bugs with changed attributes tracking when transaction gets rollback --- activerecord/lib/active_record/core.rb | 1 - activerecord/lib/active_record/transactions.rb | 6 +++ activerecord/test/cases/transactions_test.rb | 56 ++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 8808ad5a4c..a753ad7cb1 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -428,7 +428,6 @@ module ActiveRecord @destroyed_by_association = nil @new_record = true @txn = nil - @_start_transaction_state = {} @transaction_state = nil @reflects_state = [false] end diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 45313b5e75..8eafc05fdf 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -341,6 +341,7 @@ module ActiveRecord # Save the new record state and id of a record so it can be restored later if a transaction fails. def remember_transaction_record_state #:nodoc: + @_start_transaction_state ||= {} @_start_transaction_state[:id] = id if has_attribute?(self.class.primary_key) unless @_start_transaction_state.include?(:new_record) @_start_transaction_state[:new_record] = @new_record @@ -350,6 +351,8 @@ module ActiveRecord end @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) + 1 @_start_transaction_state[:frozen?] = @attributes.frozen? + @_start_transaction_state[:changed_attributes] ||= changed_attributes + @_start_transaction_state end # Clear the new record state and id of a record. @@ -368,6 +371,9 @@ module ActiveRecord @attributes = @attributes.dup if @attributes.frozen? @new_record = restore_state[:new_record] @destroyed = restore_state[:destroyed] + changed_attributes.replace(restore_state[:changed_attributes]).delete_if do |attribute, old_value| + old_value == @attributes[attribute] + end if restore_state.has_key?(:id) self.id = restore_state[:id] else diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 89dab16975..4edb200b1d 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -268,6 +268,62 @@ class TransactionTest < ActiveRecord::TestCase end end + def test_dirty_state_rollback + assert !@first.approved? + Topic.transaction do + @first.approved = true + @first.save! + raise ActiveRecord::Rollback + end + assert @first.approved + assert @first.changes["approved"] + @first.save! + assert @first.reload.approved + end + + def test_dirty_state_rollback2 + assert !@first.approved? + @first.approved = true + Topic.transaction do + @first.save! + raise ActiveRecord::Rollback + end + assert @first.changes["approved"] + assert @first.approved + @first.save! + assert @first.reload.approved + end + + def test_dirty_state_rollback3 + assert !@first.approved? + @first.approved = true + @first.save! + Topic.transaction do + @first.approved = false + @first.save! + raise ActiveRecord::Rollback + end + assert !@first.approved + assert @first.changes["approved"] + @first.save! + assert !@first.reload.approved + end + + def test_dirty_state_rollback4 + assert !@first.approved? + Topic.transaction do + @first.approved = true + @first.save! + @first.approved = false + @first.save! + raise ActiveRecord::Rollback + end + assert !@first.approved + assert !@first.changes["approved"] + @first.save! + assert !@first.reload.approved + end + def test_force_savepoint_in_nested_transaction Topic.transaction do @first.approved = true -- cgit v1.2.3 From 37c238927fbed059de3f26a90d8923fb377568a5 Mon Sep 17 00:00:00 2001 From: Arthur Neves Date: Wed, 7 May 2014 16:02:22 -0400 Subject: Keep track of dirty attrs after after rollback. [related #13166] --- activerecord/CHANGELOG.md | 6 ++++++ activerecord/lib/active_record/core.rb | 1 + activerecord/lib/active_record/transactions.rb | 2 -- activerecord/test/cases/transactions_test.rb | 22 ++++------------------ 4 files changed, 11 insertions(+), 20 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 4164b928bd..ae4a672ef1 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Keep track of dirty attributes after transaction is rollback. + + Related #13166. + + *Bogdan Gusiev* *arthurnn* + * Add support for module-level `table_name_suffix` in models. This makes `table_name_suffix` work the same way as `table_name_prefix` when diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 22870fb6c5..4571cc0786 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -505,6 +505,7 @@ module ActiveRecord @destroyed_by_association = nil @new_record = true @txn = nil + @_start_transaction_state = {} @transaction_state = nil @reflects_state = [false] end diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 33c9af3d3d..cf76d53bf7 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -341,7 +341,6 @@ module ActiveRecord # Save the new record state and id of a record so it can be restored later if a transaction fails. def remember_transaction_record_state #:nodoc: - @_start_transaction_state ||= {} @_start_transaction_state[:id] = id if has_attribute?(self.class.primary_key) unless @_start_transaction_state.include?(:new_record) @_start_transaction_state[:new_record] = @new_record @@ -352,7 +351,6 @@ module ActiveRecord @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) + 1 @_start_transaction_state[:frozen?] = @attributes.frozen? @_start_transaction_state[:changed_attributes] ||= changed_attributes - @_start_transaction_state end # Clear the new record state and id of a record. diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index bb5548a60f..7f2e830083 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -274,7 +274,7 @@ class TransactionTest < ActiveRecord::TestCase end end - def test_dirty_state_rollback + def test_rollback_when_changing_inside_transaction assert !@first.approved? Topic.transaction do @first.approved = true @@ -287,7 +287,7 @@ class TransactionTest < ActiveRecord::TestCase assert @first.reload.approved end - def test_dirty_state_rollback2 + def test_rollback_when_changing_outside_transaction assert !@first.approved? @first.approved = true Topic.transaction do @@ -300,22 +300,7 @@ class TransactionTest < ActiveRecord::TestCase assert @first.reload.approved end - def test_dirty_state_rollback3 - assert !@first.approved? - @first.approved = true - @first.save! - Topic.transaction do - @first.approved = false - @first.save! - raise ActiveRecord::Rollback - end - assert !@first.approved - assert @first.changes["approved"] - @first.save! - assert !@first.reload.approved - end - - def test_dirty_state_rollback4 + def test_rollback_when_changing_back_to_prev_stage assert !@first.approved? Topic.transaction do @first.approved = true @@ -330,6 +315,7 @@ class TransactionTest < ActiveRecord::TestCase assert !@first.reload.approved end + def test_force_savepoint_in_nested_transaction Topic.transaction do @first.approved = true -- cgit v1.2.3