Permalink
Browse files

Temporarily revert "Update after_commit and after_rollback docs and t…

…ests to use new style API with an :on options instead of on_* suffix." and "Add after_commit and after_rollback callbacks to ActiveRecord that are called after transactions either commit or rollback on all records saved or destroyed in the transaction."

This reverts commits d2a49e4 and da840d1.

[#2991]

Conflicts:

	activerecord/CHANGELOG
	activerecord/lib/active_record/transactions.rb
	activerecord/test/cases/transaction_callbacks_test.rb
  • Loading branch information...
1 parent 32d4330 commit 1b2941cba1165b0721f57524645fe378bee2a950 @jeremy jeremy committed Jun 8, 2010
View
@@ -12,8 +12,6 @@
* find_or_create_by_attr(value, ...) works when attr is protected. #4457 [Santiago Pastorino, Marc-André Lafortune]
-* New callbacks: after_commit and after_rollback. Do expensive operations like image thumbnailing after_commit instead of after_save. #2991 [Brian Durand]
-
* Serialized attributes are not converted to YAML if they are any of the formats that can be serialized to XML (like Hash, Array and Strings). [José Valim]
* Destroy uses optimistic locking. If lock_version on the record you're destroying doesn't match lock_version in the database, a StaleObjectError is raised. #1966 [Curtis Hawthorne]
@@ -122,8 +122,6 @@ def transaction(options = {})
requires_new = options[:requires_new] || !last_transaction_joinable
transaction_open = false
- @_current_transaction_records ||= []
-
begin
if block_given?
if requires_new || open_transactions == 0
@@ -134,7 +132,6 @@ def transaction(options = {})
end
increment_open_transactions
transaction_open = true
- @_current_transaction_records.push([])
end
yield
end
@@ -144,10 +141,8 @@ def transaction(options = {})
decrement_open_transactions
if open_transactions == 0
rollback_db_transaction
- rollback_transaction_records(true)
else
rollback_to_savepoint
- rollback_transaction_records(false)
end
end
raise unless database_transaction_rollback.is_a?(ActiveRecord::Rollback)
@@ -162,35 +157,20 @@ def transaction(options = {})
begin
if open_transactions == 0
commit_db_transaction
- commit_transaction_records
else
release_savepoint
- save_point_records = @_current_transaction_records.pop
- unless save_point_records.blank?
- @_current_transaction_records.push([]) if @_current_transaction_records.empty?
- @_current_transaction_records.last.concat(save_point_records)
- end
end
rescue Exception => database_transaction_rollback
if open_transactions == 0
rollback_db_transaction
- rollback_transaction_records(true)
else
rollback_to_savepoint
- rollback_transaction_records(false)
end
raise
end
end
end
- # Register a record with the current transaction so that its after_commit and after_rollback callbacks
- # can be called.
- def add_transaction_record(record)
- last_batch = @_current_transaction_records.last
- last_batch << record if last_batch
- end
-
# Begins the transaction (and turns off auto-committing).
def begin_db_transaction() end
@@ -288,42 +268,6 @@ def sanitize_limit(limit)
limit.to_i
end
end
-
- # Send a rollback message to all records after they have been rolled back. If rollback
- # is false, only rollback records since the last save point.
- def rollback_transaction_records(rollback) #:nodoc
- if rollback
- records = @_current_transaction_records.flatten
- @_current_transaction_records.clear
- else
- records = @_current_transaction_records.pop
- end
-
- unless records.blank?
- records.uniq.each do |record|
- begin
- record.rolledback!(rollback)
- rescue Exception => e
- record.logger.error(e) if record.respond_to?(:logger)
- end
- end
- end
- end
-
- # Send a commit message to all records after they have been committed.
- def commit_transaction_records #:nodoc
- records = @_current_transaction_records.flatten
- @_current_transaction_records.clear
- unless records.blank?
- records.uniq.each do |record|
- begin
- record.committed!
- rescue Exception => e
- record.logger.error(e) if record.respond_to?(:logger)
- end
- end
- end
- end
end
end
end
@@ -8,11 +8,6 @@ module Transactions
class TransactionError < ActiveRecordError # :nodoc:
end
- included do
- define_model_callbacks :commit, :commit_on_update, :commit_on_create, :commit_on_destroy, :only => :after
- define_model_callbacks :rollback, :rollback_on_update, :rollback_on_create, :rollback_on_destroy
- end
-
# Transactions are protective blocks where SQL statements are only permanent
# if they can all succeed as one atomic action. The classic example is a
# transfer between two accounts where you can only have a deposit if the
@@ -163,21 +158,6 @@ class TransactionError < ActiveRecordError # :nodoc:
# http://dev.mysql.com/doc/refman/5.0/en/savepoints.html
# for more information about savepoints.
#
- # === Callbacks
- #
- # There are two types of callbacks associated with committing and rolling back transactions:
- # +after_commit+ and +after_rollback+.
- #
- # +after_commit+ callbacks are called on every record saved or destroyed within a
- # transaction immediately after the transaction is committed. +after_rollback+ callbacks
- # are called on every record saved or destroyed within a transaction immediately after the
- # transaction or savepoint is rolled back.
- #
- # These callbacks are useful for interacting with other systems since you will be guaranteed
- # that the callback is only executed when the database is in a permanent state. For example,
- # +after_commit+ is a good spot to put in a hook to clearing a cache since clearing it from
- # within a transaction could trigger the cache to be regenerated before the database is updated.
- #
# === Caveats
#
# If you're on MySQL, then do not use DDL operations in nested transactions
@@ -225,50 +205,19 @@ def save!(*) #:nodoc:
# Reset id and @new_record if the transaction rolls back.
def rollback_active_record_state!
- remember_transaction_record_state
+ id_present = has_attribute?(self.class.primary_key)
+ previous_id = id
+ previous_new_record = new_record?
yield
rescue Exception
- restore_transaction_record_state
- raise
- ensure
- clear_transaction_record_state
- end
-
- # Call the after_commit callbacks
- def committed! #:nodoc:
- if transaction_record_state(:new_record)
- _run_commit_on_create_callbacks
- elsif transaction_record_state(:destroyed)
- _run_commit_on_destroy_callbacks
+ @new_record = previous_new_record
+ if id_present
+ self.id = previous_id
else
- _run_commit_on_update_callbacks
- end
- _run_commit_callbacks
- ensure
- clear_transaction_record_state
- end
-
- # Call the after rollback callbacks. The restore_state argument indicates if the record
- # state should be rolled back to the beginning or just to the last savepoint.
- def rolledback!(force_restore_state = false) #:nodoc:
- if transaction_record_state(:new_record)
- _run_rollback_on_create_callbacks
- elsif transaction_record_state(:destroyed)
- _run_rollback_on_destroy_callbacks
- else
- _run_rollback_on_update_callbacks
- end
- _run_rollback_callbacks
- ensure
- restore_transaction_record_state(force_restore_state)
- end
-
- # Add the record to the current transaction so that the :after_rollback and :after_commit callbacks
- # can be called.
- def add_to_transaction
- if self.class.connection.add_transaction_record(self)
- remember_transaction_record_state
+ @attributes.delete(self.class.primary_key)
+ @attributes_cache.delete(self.class.primary_key)
end
+ raise
end
# Executes +method+ within a transaction and captures its return value as a
@@ -280,59 +229,10 @@ def add_to_transaction
def with_transaction_returning_status
status = nil
self.class.transaction do
- add_to_transaction
status = yield
raise ActiveRecord::Rollback unless status
end
status
end
-
- protected
-
- # 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 ||= {}
- unless @_start_transaction_state.include?(:new_record)
- @_start_transaction_state[:id] = id if has_attribute?(self.class.primary_key)
- @_start_transaction_state[:new_record] = @new_record
- end
- unless @_start_transaction_state.include?(:destroyed)
- @_start_transaction_state[:destroyed] = @new_record
- end
- @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) + 1
- end
-
- # Clear the new record state and id of a record.
- def clear_transaction_record_state #:nodoc
- if defined?(@_start_transaction_state)
- @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) - 1
- remove_instance_variable(:@_start_transaction_state) if @_start_transaction_state[:level] < 1
- end
- end
-
- # Restore the new record state and id of a record that was previously saved by a call to save_record_state.
- def restore_transaction_record_state(force = false) #:nodoc
- if defined?(@_start_transaction_state)
- @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) - 1
- if @_start_transaction_state[:level] < 1
- restore_state = remove_instance_variable(:@_start_transaction_state)
- if restore_state
- @new_record = restore_state[:new_record]
- @destroyed = restore_state[:destroyed]
- if restore_state[:id]
- self.id = restore_state[:id]
- else
- @attributes.delete(self.class.primary_key)
- @attributes_cache.delete(self.class.primary_key)
- end
- end
- end
- end
- end
-
- # Determine if a record was created or destroyed in a transaction. State should be one of :new_record or :destroyed.
- def transaction_record_state(state) #:nodoc
- @_start_transaction_state[state] if defined?(@_start_transaction_state)
- end
end
end
Oops, something went wrong.

2 comments on commit 1b2941c

Contributor

nragaz replied Jun 8, 2010

Can you explain the reasoning behind this?

These callbacks seemed useful. Is there a replacement?

Owner

jeremy replied Jun 8, 2010

Tests were failing; callbacks running at the wrong times. See http://rails.lighthouseapp.com/projects/8994/tickets/2991-after-transaction-patch

Please sign in to comment.