Current transaction records retained by transactional fixtures (master) #3300

Closed
wants to merge 2 commits into
from
Jump to file or symbol
Failed to load files and symbols.
+56 −4
Split
@@ -162,14 +162,19 @@ def supports_statement_cache?
# end # RELEASE SAVEPOINT active_record_1 <--- BOOM! database error!
# end
def transaction(options = {})
- options.assert_valid_keys :requires_new, :joinable
+ options.assert_valid_keys :requires_new, :joinable, :remember_record_state
last_transaction_joinable = defined?(@transaction_joinable) ? @transaction_joinable : nil
if options.has_key?(:joinable)
@transaction_joinable = options[:joinable]
else
@transaction_joinable = true
end
+ if options.has_key?(:remember_record_state)
+ remember_record_state = options[:remember_record_state]
+ else
+ remember_record_state = true
+ end
@carlosantoniodasilva

carlosantoniodasilva Nov 17, 2012

Owner

This can be simplified with options.fetch

requires_new = options[:requires_new] || !last_transaction_joinable
transaction_open = false
@@ -185,7 +190,7 @@ def transaction(options = {})
end
increment_open_transactions
transaction_open = true
- @_current_transaction_records.push([])
+ @_current_transaction_records.push(remember_record_state ? [] : nil)
end
yield
end
@@ -217,8 +222,7 @@ def transaction(options = {})
else
release_savepoint
save_point_records = @_current_transaction_records.pop
- unless save_point_records.blank?
- @_current_transaction_records.push([]) if @_current_transaction_records.empty?
+ unless save_point_records.blank? || @_current_transaction_records.empty?
@_current_transaction_records.last.concat(save_point_records)
end
end
@@ -335,6 +335,31 @@ def test_destroy_just_kidding
end
end
+class TransactionalFixturesTest < ActiveRecord::TestCase
+ self.use_instantiated_fixtures = true
+ self.use_transactional_fixtures = true
+
+ fixtures :topics
+
+ def test_records_saved_in_fixture_transaction_not_recorded
+ @first.update_attributes!(:title => "New title")
+ assert !Topic.connection.instance_variable_get("@_current_transaction_records").flatten.include?(@first)
+ end
+
+ def test_records_saved_in_explicit_transaction_recorded
+ Topic.transaction(:requires_new => true) do
+ @first.update_attributes!(:title => "New title")
+ assert Topic.connection.instance_variable_get("@_current_transaction_records").flatten.include?(@first)
+ end
+ Topic.transaction do
+ @second.update_attributes!(:title => "New title")
+ assert Topic.connection.instance_variable_get("@_current_transaction_records").flatten.include?(@second)
+ end
+ assert !Topic.connection.instance_variable_get("@_current_transaction_records").flatten.include?(@first)
+ assert !Topic.connection.instance_variable_get("@_current_transaction_records").flatten.include?(@second)
+ end
+end
+
class MultipleFixturesTest < ActiveRecord::TestCase
fixtures :topics
fixtures :developers, :accounts
@@ -389,6 +389,29 @@ def test_restore_active_record_state_for_all_records_in_a_transaction
assert !@second.destroyed?, 'not destroyed'
end
+ def test_optional_no_restore_active_record_state_so_records_not_retained_in_memory
+ topic_1 = Topic.new(:title => 'test_1')
+ Topic.transaction(:remember_record_state => false) do
+ assert topic_1.save
+ @first.destroy
+ assert topic_1.persisted?, 'persisted'
+ assert_not_nil topic_1.id
+ assert @first.destroyed?, 'destroyed'
+ raise ActiveRecord::Rollback
+ end
+
+ assert topic_1.persisted?, 'state was rolled back, so record must have been retained'
+ assert_not_nil topic_1.id
+ assert_raises(ActiveRecord::RecordNotFound) do
+ topic_1.reload
+ flunk 'database was not rolled back'
+ end
+ assert @first.destroyed?, 'state was rolled back, so record must have been retained'
+ assert_nothing_raised do
+ Topic.find(@first.id)
+ end
+ end
+
if current_adapter?(:PostgreSQLAdapter) && defined?(PGconn::PQTRANS_IDLE)
def test_outside_transaction_works
assert Topic.connection.outside_transaction?