Rollback #new_record? and #id values for created records that rollback in an after_save callback. Closes #6910 [Ben Curren]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@5830 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

activerecord/CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Rollback #new_record? and #id values for created records that rollback in an after_save callback. Closes #6910 [Ben Curren]
+
 * Pushing a record on an association collection doesn't unnecessarily load all the associated records.  [Obie Fernandez, Jeremy Kemper]
 
 * Oracle: fix connection reset failure.  #6846 [leonlleslie]

activerecord/lib/active_record/transactions.rb

@@ -126,11 +126,28 @@ module ActiveRecord
     end
 
     def save_with_transactions(perform_validation = true) #:nodoc:
-      transaction { save_without_transactions(perform_validation) }
+      rollback_active_record_state { transaction { save_without_transactions(perform_validation) } }
     end
 
     def save_with_transactions! #:nodoc:
-      transaction { save_without_transactions! }
+      rollback_active_record_state { transaction { save_without_transactions! } }
+    end
+    
+    # stores the current id and @new_record values so that they are reset
+    # after rolling the transaction back.
+    def rollback_active_record_state
+      previous_new_record = @new_record
+      previous_id = self.id
+      response = yield
+    rescue
+      response = false
+      raise
+    ensure
+      unless response
+        @new_record = previous_new_record
+        self.id = previous_id
+      end
+      response
     end
   end
 end

activerecord/test/transactions_test.rb

@@ -121,6 +121,36 @@ class TransactionTest < Test::Unit::TestCase
       remove_exception_raising_after_save_callback_to_topic
     end
   end
+  
+  def test_callback_rollback_in_create
+    new_topic = Topic.new(
+      :title => "A new topic",
+      :author_name => "Ben",
+      :author_email_address => "ben@example.com",
+      :written_on => "2003-07-16t15:28:11.2233+01:00",
+      :last_read => "2004-04-15",
+      :bonus_time => "2005-01-30t15:28:00.00+01:00",
+      :content => "Have a nice day",
+      :approved => false)
+    new_record_snapshot = new_topic.new_record?
+    id_snapshot = new_topic.id
+    
+    # Make sure the second save gets the after_create callback called.
+    2.times do
+      begin
+        add_exception_raising_after_create_callback_to_topic
+        new_topic.approved = true
+        new_topic.save
+        flunk
+      rescue => e
+        assert_equal "Make the transaction rollback", e.message
+        assert_equal new_record_snapshot, new_topic.new_record?, "The topic should have its old new_record value"
+        assert_equal id_snapshot, new_topic.id, "The topic should have its old id"
+      ensure
+        remove_exception_raising_after_create_callback_to_topic
+      end
+    end
+  end
 
   def test_nested_explicit_transactions
     Topic.transaction do
@@ -144,6 +174,14 @@ class TransactionTest < Test::Unit::TestCase
     def remove_exception_raising_after_save_callback_to_topic
       Topic.class_eval { remove_method :after_save }
     end
+    
+    def add_exception_raising_after_create_callback_to_topic
+      Topic.class_eval { def after_create() raise "Make the transaction rollback" end }
+    end
+    
+    def remove_exception_raising_after_create_callback_to_topic
+      Topic.class_eval { remove_method :after_create }
+    end
 end
 
 if current_adapter?(:PostgreSQLAdapter)