From 100f8b2dfc9d547b23804b648ad0dba748faa99b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Santiago=20Pastorino=20and=20Jos=C3=A9=20Ignacio=20Costa?=
 <santiago+jose@wyeworks.com>
Date: Mon, 7 Jun 2010 22:21:28 -0300
Subject: update docs before_validation_on_create => before_validation(:on =>
 :create)

---
 activerecord/lib/active_record/callbacks.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb
index 498836aca4..44fee12001 100644
--- a/activerecord/lib/active_record/callbacks.rb
+++ b/activerecord/lib/active_record/callbacks.rb
@@ -31,7 +31,7 @@ module ActiveRecord
   #   class CreditCard < ActiveRecord::Base
   #     # Strip everything but digits, so the user can specify "555 234 34" or
   #     # "5552-3434" or both will mean "55523434"
-  #     def before_validation_on_create
+  #     before_validation(:on => :create) do
   #       self.number = number.gsub(/[^0-9]/, "") if attribute_present?("number")
   #     end
   #   end
-- 
cgit v1.2.3


From ab2877cbe89e266ee986fc12e603abd93ac017ad Mon Sep 17 00:00:00 2001
From: Emilio Tagua <miloops@gmail.com>
Date: Tue, 8 Jun 2010 15:07:51 -0400
Subject: Add parentheses to avoid warnings when running AR tests.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: José Valim <jose.valim@gmail.com>
---
 activerecord/test/cases/adapter_test.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 0152b7be2a..fc08c2178a 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -145,13 +145,13 @@ class AdapterTest < ActiveRecord::TestCase
 
   def test_add_limit_offset_should_sanitize_sql_injection_for_limit_without_comas
     sql_inject = "1 select * from schema"
-    assert_no_match /schema/, @connection.add_limit_offset!("", :limit=>sql_inject)
-    assert_no_match /schema/, @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7)
+    assert_no_match(/schema/, @connection.add_limit_offset!("", :limit=>sql_inject))
+    assert_no_match(/schema/, @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7))
   end
 
   def test_add_limit_offset_should_sanitize_sql_injection_for_limit_with_comas
     sql_inject = "1, 7 procedure help()"
-    assert_no_match /procedure/, @connection.add_limit_offset!("", :limit=>sql_inject)
-    assert_no_match /procedure/, @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7)
+    assert_no_match(/procedure/, @connection.add_limit_offset!("", :limit=>sql_inject))
+    assert_no_match(/procedure/, @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7))
   end
 end
-- 
cgit v1.2.3


From 0e9b9d59859efa46a82b56e0715784fa52656650 Mon Sep 17 00:00:00 2001
From: Andrew Bloomgarden <stalkingtiger@gmail.com>
Date: Tue, 1 Jun 2010 22:47:34 -0700
Subject: Fix ActiveRecord::Base.compute_type swallowing NoMethodError. [#4751
 state:resolved]

Signed-off-by: David Heinemeier Hansson <david@loudthinking.com>
---
 activerecord/CHANGELOG                 |  2 ++
 activerecord/lib/active_record/base.rb |  4 +++-
 activerecord/test/cases/base_test.rb   | 17 +++++++++++++++++
 3 files changed, 22 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 348248e849..e47d291eb3 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *Rails 3.0.0 [beta 4] (June 8th, 2010)*
 
+* Fixed that ActiveRecord::Base.compute_type would swallow NoMethodError #4751 [Andrew Bloomgarden, Andrew White]
+
 * Add index length support for MySQL. #1852 [Emili Parreno, Pratik Naik]
 
   Example:
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index aa2826fb33..7cff6d9f1a 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1219,7 +1219,9 @@ module ActiveRecord #:nodoc:
               begin
                 constant = candidate.constantize
                 return constant if candidate == constant.to_s
-              rescue NameError
+              rescue NameError => e
+                # We don't want to swallow NoMethodError < NameError errors
+                raise e unless e.instance_of?(NameError)
               rescue ArgumentError
               end
             end
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index 36c572b5e7..5c175de6d4 100755
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -2334,6 +2334,23 @@ class BasicsTest < ActiveRecord::TestCase
     assert !Minimalistic.new.freeze.dup.frozen?
   end
 
+  def test_compute_type_success
+    assert_equal Author, ActiveRecord::Base.send(:compute_type, 'Author')
+  end
+
+  def test_compute_type_nonexistent_constant
+    assert_raises NameError do
+      ActiveRecord::Base.send :compute_type, 'NonexistentModel'
+    end
+  end
+
+  def test_compute_type_no_method_error
+    String.any_instance.stubs(:constantize).raises(NoMethodError)
+    assert_raises NoMethodError do
+      ActiveRecord::Base.send :compute_type, 'InvalidModel'
+    end
+  end
+
   protected
     def with_env_tz(new_tz = 'US/Eastern')
       old_tz, ENV['TZ'] = ENV['TZ'], new_tz
-- 
cgit v1.2.3


From 4b4a548a60236ccfb68c91f53386459bcb1751ab Mon Sep 17 00:00:00 2001
From: Emilio Tagua <miloops@gmail.com>
Date: Tue, 8 Jun 2010 16:32:10 -0400
Subject: Avoid PostgreSQL and MySQL tests warnings.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: José Valim <jose.valim@gmail.com>
---
 activerecord/test/cases/active_schema_test_mysql.rb      | 8 +++++---
 activerecord/test/cases/active_schema_test_postgresql.rb | 1 +
 2 files changed, 6 insertions(+), 3 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/active_schema_test_mysql.rb b/activerecord/test/cases/active_schema_test_mysql.rb
index 3526f49afd..d7431e5158 100644
--- a/activerecord/test/cases/active_schema_test_mysql.rb
+++ b/activerecord/test/cases/active_schema_test_mysql.rb
@@ -4,6 +4,7 @@ class ActiveSchemaTest < ActiveRecord::TestCase
   def setup
     ActiveRecord::ConnectionAdapters::MysqlAdapter.class_eval do
       alias_method :execute_without_stub, :execute
+      remove_method :execute
       def execute(sql, name = nil) return sql end
     end
   end
@@ -66,7 +67,7 @@ class ActiveSchemaTest < ActiveRecord::TestCase
     assert_equal "DROP TABLE `otherdb`.`people`", drop_table('otherdb.people')
   end
 
-  def test_add_timestamps 
+  def test_add_timestamps
     with_real_execute do
       begin
         ActiveRecord::Base.connection.create_table :delete_me do |t|
@@ -79,8 +80,8 @@ class ActiveSchemaTest < ActiveRecord::TestCase
       end
     end
   end
-  
-  def test_remove_timestamps 
+
+  def test_remove_timestamps
     with_real_execute do
       begin
         ActiveRecord::Base.connection.create_table :delete_me do |t|
@@ -106,6 +107,7 @@ class ActiveSchemaTest < ActiveRecord::TestCase
     ensure
       #before finishing, we restore the alias to the mock-up method
       ActiveRecord::ConnectionAdapters::MysqlAdapter.class_eval do
+        remove_method :execute
         alias_method :execute, :execute_with_stub
       end
     end
diff --git a/activerecord/test/cases/active_schema_test_postgresql.rb b/activerecord/test/cases/active_schema_test_postgresql.rb
index af80f724f2..4f04c6735c 100644
--- a/activerecord/test/cases/active_schema_test_postgresql.rb
+++ b/activerecord/test/cases/active_schema_test_postgresql.rb
@@ -4,6 +4,7 @@ class PostgresqlActiveSchemaTest < Test::Unit::TestCase
   def setup
     ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.class_eval do
       alias_method :real_execute, :execute
+      remove_method :execute
       def execute(sql, name = nil) sql end
     end
   end
-- 
cgit v1.2.3


From 87cc3d556948f4cb644091c98969c03e319c864e Mon Sep 17 00:00:00 2001
From: Jeremy Kemper <jeremy@bitsweat.net>
Date: Tue, 8 Jun 2010 16:57:46 -0400
Subject: Clarify Fixture#key_ and value_list

---
 activerecord/lib/active_record/fixtures.rb | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb
index 8099aaa7f7..82270c56b3 100644
--- a/activerecord/lib/active_record/fixtures.rb
+++ b/activerecord/lib/active_record/fixtures.rb
@@ -787,16 +787,14 @@ class Fixture #:nodoc:
   end
 
   def key_list
-    columns = @fixture.keys.collect{ |column_name| @connection.quote_column_name(column_name) }
-    columns.join(", ")
+    @fixture.keys.map { |column_name| @connection.quote_column_name(column_name) }.join(', ')
   end
 
   def value_list
-    list = @fixture.inject([]) do |fixtures, (key, value)|
-      col = model_class.columns_hash[key] if model_class.respond_to?(:ancestors) && model_class.ancestors.include?(ActiveRecord::Base)
-      fixtures << @connection.quote(value, col).gsub('[^\]\\n', "\n").gsub('[^\]\\r', "\r")
-    end
-    list * ', '
+    cols = (model_class && model_class < ActiveRecord::Base) ? model_class.columns_hash : {}
+    @fixture.map do |key, value|
+      @connection.quote(value, cols[key]).gsub('[^\]\\n', "\n").gsub('[^\]\\r', "\r")
+    end.join(', ')
   end
 
   def find
-- 
cgit v1.2.3


From b07073924002fd56ac5b63b24cb9318b3dee45c4 Mon Sep 17 00:00:00 2001
From: Jeremy Kemper <jeremy@bitsweat.net>
Date: Tue, 8 Jun 2010 16:59:06 -0400
Subject: Revert "Temporarily revert "Update after_commit and after_rollback
 docs and tests 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 commit 1b2941cba1165b0721f57524645fe378bee2a950.

[#2991]
---
 activerecord/CHANGELOG                             |   2 +
 .../abstract/database_statements.rb                |  56 +++++
 activerecord/lib/active_record/transactions.rb     | 118 +++++++++-
 .../test/cases/transaction_callbacks_test.rb       | 240 +++++++++++++++++++++
 activerecord/test/cases/transactions_test.rb       |  33 +++
 5 files changed, 440 insertions(+), 9 deletions(-)
 create mode 100644 activerecord/test/cases/transaction_callbacks_test.rb

(limited to 'activerecord')

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index e47d291eb3..7d5e550a7c 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -14,6 +14,8 @@
 
 * 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]
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
index 0c87e052c4..b9fb452eee 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
@@ -122,6 +122,8 @@ module ActiveRecord
         requires_new = options[:requires_new] || !last_transaction_joinable
 
         transaction_open = false
+        @_current_transaction_records ||= []
+
         begin
           if block_given?
             if requires_new || open_transactions == 0
@@ -132,6 +134,7 @@ module ActiveRecord
               end
               increment_open_transactions
               transaction_open = true
+              @_current_transaction_records.push([])
             end
             yield
           end
@@ -141,8 +144,10 @@ module ActiveRecord
             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)
@@ -157,20 +162,35 @@ module ActiveRecord
           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
 
@@ -268,6 +288,42 @@ module ActiveRecord
             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
diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb
index 3f2c1911e7..5a8e2ce880 100644
--- a/activerecord/lib/active_record/transactions.rb
+++ b/activerecord/lib/active_record/transactions.rb
@@ -8,6 +8,11 @@ module ActiveRecord
     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
@@ -158,6 +163,21 @@ module ActiveRecord
     # 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
@@ -205,19 +225,50 @@ module ActiveRecord
 
     # Reset id and @new_record if the transaction rolls back.
     def rollback_active_record_state!
-      id_present = has_attribute?(self.class.primary_key)
-      previous_id = id
-      previous_new_record = new_record?
+      remember_transaction_record_state
       yield
     rescue Exception
-      @new_record = previous_new_record
-      if id_present
-        self.id = previous_id
+      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
       else
-        @attributes.delete(self.class.primary_key)
-        @attributes_cache.delete(self.class.primary_key)
+        _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
       end
-      raise
     end
 
     # Executes +method+ within a transaction and captures its return value as a
@@ -229,10 +280,59 @@ module ActiveRecord
     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
diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb
new file mode 100644
index 0000000000..ebc16653cb
--- /dev/null
+++ b/activerecord/test/cases/transaction_callbacks_test.rb
@@ -0,0 +1,240 @@
+require "cases/helper"
+require 'models/topic'
+require 'models/reply'
+
+class TransactionCallbacksTest < ActiveRecord::TestCase
+  self.use_transactional_fixtures = false
+  fixtures :topics
+
+  class TopicWithCallbacks < ActiveRecord::Base
+    set_table_name :topics
+
+    after_commit{|record| record.send(:do_after_commit, nil)}
+    after_commit(:on => :create){|record| record.send(:do_after_commit, :create)}
+    after_commit(:on => :update){|record| record.send(:do_after_commit, :update)}
+    after_commit(:on => :destroy){|record| record.send(:do_after_commit, :destroy)}
+    after_rollback{|record| record.send(:do_after_rollback, nil)}
+    after_rollback(:on => :create){|record| record.send(:do_after_rollback, :create)}
+    after_rollback(:on => :update){|record| record.send(:do_after_rollback, :update)}
+    after_rollback(:on => :destroy){|record| record.send(:do_after_rollback, :destroy)}
+
+    def history
+      @history ||= []
+    end
+
+    def after_commit_block(on = nil, &block)
+      @after_commit ||= {}
+      @after_commit[on] ||= []
+      @after_commit[on] << block
+    end
+
+    def after_rollback_block(on = nil, &block)
+      @after_rollback ||= {}
+      @after_rollback[on] ||= []
+      @after_rollback[on] << block
+    end
+
+    def do_after_commit(on)
+      blocks = @after_commit[on] if defined?(@after_commit)
+      blocks.each{|b| b.call(self)} if blocks
+    end
+
+    def do_after_rollback(on)
+      blocks = @after_rollback[on] if defined?(@after_rollback)
+      blocks.each{|b| b.call(self)} if blocks
+    end
+  end
+
+  def setup
+    @first, @second = TopicWithCallbacks.find(1, 3).sort_by { |t| t.id }
+  end
+
+  def test_call_after_commit_after_transaction_commits
+    @first.after_commit_block{|r| r.history << :after_commit}
+    @first.after_rollback_block{|r| r.history << :after_rollback}
+
+    @first.save!
+    assert_equal [:after_commit], @first.history
+  end
+
+  def test_only_call_after_commit_on_update_after_transaction_commits_for_existing_record
+    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
+    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
+    @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
+    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
+    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
+    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+
+    @first.save!
+    assert_equal [:commit_on_update], @first.history
+  end
+
+  def test_only_call_after_commit_on_destroy_after_transaction_commits_for_destroyed_record
+    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
+    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
+    @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
+    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
+    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
+    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+
+    @first.destroy
+    assert_equal [:commit_on_destroy], @first.history
+  end
+
+  def test_only_call_after_commit_on_create_after_transaction_commits_for_new_record
+    @new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today)
+    @new_record.after_commit_block(:create){|r| r.history << :commit_on_create}
+    @new_record.after_commit_block(:update){|r| r.history << :commit_on_update}
+    @new_record.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
+    @new_record.after_rollback_block(:create){|r| r.history << :rollback_on_create}
+    @new_record.after_rollback_block(:update){|r| r.history << :rollback_on_update}
+    @new_record.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+
+    @new_record.save!
+    assert_equal [:commit_on_create], @new_record.history
+  end
+
+  def test_call_after_rollback_after_transaction_rollsback
+    @first.after_commit_block{|r| r.history << :after_commit}
+    @first.after_rollback_block{|r| r.history << :after_rollback}
+
+    Topic.transaction do
+      @first.save!
+      raise ActiveRecord::Rollback
+    end
+
+    assert_equal [:after_rollback], @first.history
+  end
+
+  def test_only_call_after_rollback_on_update_after_transaction_rollsback_for_existing_record
+    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
+    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
+    @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
+    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
+    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
+    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+
+    Topic.transaction do
+      @first.save!
+      raise ActiveRecord::Rollback
+    end
+
+    assert_equal [:rollback_on_update], @first.history
+  end
+
+  def test_only_call_after_rollback_on_destroy_after_transaction_rollsback_for_destroyed_record
+    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
+    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
+    @first.after_commit_block(:destroy){|r| r.history << :commit_on_update}
+    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
+    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
+    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+
+    Topic.transaction do
+      @first.destroy
+      raise ActiveRecord::Rollback
+    end
+
+    assert_equal [:rollback_on_destroy], @first.history
+  end
+
+  def test_only_call_after_rollback_on_create_after_transaction_rollsback_for_new_record
+    @new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today)
+    @new_record.after_commit_block(:create){|r| r.history << :commit_on_create}
+    @new_record.after_commit_block(:update){|r| r.history << :commit_on_update}
+    @new_record.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
+    @new_record.after_rollback_block(:create){|r| r.history << :rollback_on_create}
+    @new_record.after_rollback_block(:update){|r| r.history << :rollback_on_update}
+    @new_record.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+
+    Topic.transaction do
+      @new_record.save!
+      raise ActiveRecord::Rollback
+    end
+
+    assert_equal [:rollback_on_create], @new_record.history
+  end
+
+  def test_call_after_rollback_when_commit_fails
+    @first.connection.class.send(:alias_method, :real_method_commit_db_transaction, :commit_db_transaction)
+    begin
+      @first.connection.class.class_eval do
+        def commit_db_transaction; raise "boom!"; end
+      end
+
+      @first.after_commit_block{|r| r.history << :after_commit}
+      @first.after_rollback_block{|r| r.history << :after_rollback}
+
+      assert !@first.save rescue nil
+      assert_equal [:after_rollback], @first.history
+    ensure
+      @first.connection.class.send(:remove_method, :commit_db_transaction)
+      @first.connection.class.send(:alias_method, :commit_db_transaction, :real_method_commit_db_transaction)
+    end
+  end
+
+  def test_only_call_after_rollback_on_records_rolled_back_to_a_savepoint
+    def @first.rollbacks(i=0); @rollbacks ||= 0; @rollbacks += i if i; end
+    def @first.commits(i=0); @commits ||= 0; @commits += i if i; end
+    @first.after_rollback_block{|r| r.rollbacks(1)}
+    @first.after_commit_block{|r| r.commits(1)}
+
+    def @second.rollbacks(i=0); @rollbacks ||= 0; @rollbacks += i if i; end
+    def @second.commits(i=0); @commits ||= 0; @commits += i if i; end
+    @second.after_rollback_block{|r| r.rollbacks(1)}
+    @second.after_commit_block{|r| r.commits(1)}
+
+    Topic.transaction do
+      @first.save!
+      Topic.transaction(:requires_new => true) do
+        @second.save!
+        raise ActiveRecord::Rollback
+      end
+    end
+
+    assert_equal 1, @first.commits
+    assert_equal 0, @first.rollbacks
+    assert_equal 0, @second.commits
+    assert_equal 1, @second.rollbacks
+  end
+
+  def test_only_call_after_rollback_on_records_rolled_back_to_a_savepoint_when_release_savepoint_fails
+    def @first.rollbacks(i=0); @rollbacks ||= 0; @rollbacks += i if i; end
+    def @first.commits(i=0); @commits ||= 0; @commits += i if i; end
+
+    @first.after_rollback_block{|r| r.rollbacks(1)}
+    @first.after_commit_block{|r| r.commits(1)}
+
+    Topic.transaction do
+      @first.save
+      Topic.transaction(:requires_new => true) do
+        @first.save!
+        raise ActiveRecord::Rollback
+      end
+      Topic.transaction(:requires_new => true) do
+        @first.save!
+        raise ActiveRecord::Rollback
+      end
+    end
+
+    assert_equal 1, @first.commits
+    assert_equal 2, @first.rollbacks
+  end
+
+  def test_after_transaction_callbacks_should_not_raise_errors
+    def @first.last_after_transaction_error=(e); @last_transaction_error = e; end
+    def @first.last_after_transaction_error; @last_transaction_error; end
+    @first.after_commit_block{|r| r.last_after_transaction_error = :commit; raise "fail!";}
+    @first.after_rollback_block{|r| r.last_after_transaction_error = :rollback; raise "fail!";}
+
+    @first.save!
+    assert_equal :commit, @first.last_after_transaction_error
+
+    Topic.transaction do
+      @first.save!
+      raise ActiveRecord::Rollback
+    end
+
+    assert_equal :rollback, @first.last_after_transaction_error
+  end
+end
diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb
index 00f3b527d7..958a4e4f94 100644
--- a/activerecord/test/cases/transactions_test.rb
+++ b/activerecord/test/cases/transactions_test.rb
@@ -320,6 +320,33 @@ class TransactionTest < ActiveRecord::TestCase
     end
   end
 
+  def test_restore_active_record_state_for_all_records_in_a_transaction
+    topic_1 = Topic.new(:title => 'test_1')
+    topic_2 = Topic.new(:title => 'test_2')
+    Topic.transaction do
+      assert topic_1.save
+      assert topic_2.save
+      @first.save
+      @second.destroy
+      assert_equal false, topic_1.new_record?
+      assert_not_nil topic_1.id
+      assert_equal false, topic_2.new_record?
+      assert_not_nil topic_2.id
+      assert_equal false, @first.new_record?
+      assert_not_nil @first.id
+      assert_equal true, @second.destroyed?
+      raise ActiveRecord::Rollback
+    end
+
+    assert_equal true, topic_1.new_record?
+    assert_nil topic_1.id
+    assert_equal true, topic_2.new_record?
+    assert_nil topic_2.id
+    assert_equal false, @first.new_record?
+    assert_not_nil @first.id
+    assert_equal false, @second.destroyed?
+  end
+
   if current_adapter?(:PostgreSQLAdapter) && defined?(PGconn::PQTRANS_IDLE)
     def test_outside_transaction_works
       assert Topic.connection.outside_transaction?
@@ -382,6 +409,12 @@ class TransactionTest < ActiveRecord::TestCase
   end
 
   private
+    def define_callback_method(callback_method)
+      define_method(callback_method) do
+        self.history << [callback_method, :method]
+      end
+    end
+
     def add_exception_raising_after_save_callback_to_topic
       Topic.class_eval <<-eoruby, __FILE__, __LINE__ + 1
         remove_method(:after_save_for_transaction)
-- 
cgit v1.2.3


From 2500e6af6634c984f7d2b567403de1c4c544ff91 Mon Sep 17 00:00:00 2001
From: Brian Durand <brian@embellishedvisions.com>
Date: Tue, 8 Jun 2010 14:41:42 -0500
Subject: Make logic for after_commit and after_rollback :on option work like
 it does for validation callbacks.

[#2991 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
---
 activerecord/lib/active_record/transactions.rb | 51 +++++++++++++++++---------
 1 file changed, 33 insertions(+), 18 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb
index 5a8e2ce880..620758f5af 100644
--- a/activerecord/lib/active_record/transactions.rb
+++ b/activerecord/lib/active_record/transactions.rb
@@ -9,8 +9,7 @@ module ActiveRecord
     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
+      define_callbacks :commit, :rollback, :terminator => "result == false", :scope => [:kind, :name]
     end
 
     # Transactions are protective blocks where SQL statements are only permanent
@@ -77,7 +76,7 @@ module ActiveRecord
     #
     # Both +save+ and +destroy+ come wrapped in a transaction that ensures
     # that whatever you do in validations or callbacks will happen under its
-    # protected cover. So you can use validations to check for values that 
+    # protected cover. So you can use validations to check for values that
     # the transaction depends on or you can raise exceptions in the callbacks
     # to rollback, including <tt>after_*</tt> callbacks.
     #
@@ -202,6 +201,24 @@ module ActiveRecord
         # See the ConnectionAdapters::DatabaseStatements#transaction API docs.
         connection.transaction(options, &block)
       end
+
+      def after_commit(*args, &block)
+        options = args.last
+        if options.is_a?(Hash) && options[:on]
+          options[:if] = Array.wrap(options[:if])
+          options[:if] << "transaction_include_action?(:#{options[:on]})"
+        end
+        set_callback(:commit, :after, *args, &block)
+      end
+
+      def after_rollback(*args, &block)
+        options = args.last
+        if options.is_a?(Hash) && options[:on]
+          options[:if] = Array.wrap(options[:if])
+          options[:if] << "transaction_include_action?(:#{options[:on]})"
+        end
+        set_callback(:rollback, :after, *args, &block)
+      end
     end
 
     # See ActiveRecord::Transactions::ClassMethods for detailed documentation.
@@ -236,13 +253,6 @@ module ActiveRecord
 
     # 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
-      else
-        _run_commit_on_update_callbacks
-      end
       _run_commit_callbacks
     ensure
       clear_transaction_record_state
@@ -251,13 +261,6 @@ module ActiveRecord
     # 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)
@@ -297,7 +300,7 @@ module ActiveRecord
         @_start_transaction_state[:new_record] = @new_record
       end
       unless @_start_transaction_state.include?(:destroyed)
-        @_start_transaction_state[:destroyed] = @new_record
+        @_start_transaction_state[:destroyed] = @destroyed
       end
       @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) + 1
     end
@@ -334,5 +337,17 @@ module ActiveRecord
     def transaction_record_state(state) #:nodoc
       @_start_transaction_state[state] if defined?(@_start_transaction_state)
     end
+
+    # Determine if a transaction included an action for :create, :update, or :destroy. Used in filtering callbacks.
+    def transaction_include_action?(action) #:nodoc
+      case action
+      when :create
+        transaction_record_state(:new_record)
+      when :destroy
+        destroyed?
+      when :update
+        !(transaction_record_state(:new_record) || destroyed?)
+      end
+    end
   end
 end
-- 
cgit v1.2.3