Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix rollback of frozen records #18200

Merged
merged 2 commits into from
Mar 7, 2015
Merged

Fix rollback of frozen records #18200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8313797
Fix rollback of frozen records
Dec 25, 2014
caae79a
Fix transaction state for unsynced records when entering transaction
Mar 4, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion activerecord/lib/active_record/transactions.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ def add_to_transaction
if has_transactional_callbacks?
self.class.connection.add_transaction_record(self)
else
sync_with_transaction_state
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are always going to be cleaning the state before adding the record to a new transaction, why do we need to hold multiple levels on _start_transaction_state ?
I wonder if this is breaking something else we dont have tests for it. Something around .destroy on a multi level transaction.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It won't clean up state unless two conditions are met:

So it looks like a safe method to call. It will do appropriate job when necessary.

set_transaction_state(self.class.connection.transaction_state)
end
remember_transaction_record_state
Expand Down Expand Up @@ -413,13 +414,14 @@ def restore_transaction_record_state(force = false) #:nodoc:
transaction_level = (@_start_transaction_state[:level] || 0) - 1
if transaction_level < 1 || force
restore_state = @_start_transaction_state
thaw unless restore_state[:frozen?]
thaw
@new_record = restore_state[:new_record]
@destroyed = restore_state[:destroyed]
pk = self.class.primary_key
if pk && read_attribute(pk) != restore_state[:id]
write_attribute(pk, restore_state[:id])
end
freeze if restore_state[:frozen?]
end
end
end
Expand Down
18 changes: 18 additions & 0 deletions activerecord/test/cases/transactions_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,24 @@ def test_restore_frozen_state_after_double_destroy
assert_not topic.frozen?
end

def test_rollback_of_frozen_records
topic = Topic.create.freeze
Topic.transaction do
topic.destroy
raise ActiveRecord::Rollback
end
assert topic.frozen?, 'frozen'
end

def test_rollback_for_freshly_persisted_records
topic = Topic.create
Topic.transaction do
topic.destroy
raise ActiveRecord::Rollback
end
assert topic.persisted?, 'persisted'
end

def test_sqlite_add_column_in_transaction
return true unless current_adapter?(:SQLite3Adapter)

Expand Down