Fix new_record? and id rollback. Closes #6910.

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

3 files changed, 18 insertions, 17 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 720f203299..435aeac9b9 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -11,7 +11,7 @@
 
 * Bring the sybase adapter up to scratch for 1.2 release. [jsheets]
 
-* Rollback #new_record? and #id values for created records that rollback in an after_save callback. Closes #6910 [Ben Curren]
+* Rollback new_record? and id when an exception is raised in a save callback.  #6910 [Ben Curren, outerim]
 
 * Pushing a record on an association collection doesn't unnecessarily load all the associated records.  [Obie Fernandez, Jeremy Kemper]
 
diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb
index 5ee644c0b4..77a8a14a2a 100644
--- a/activerecord/lib/active_record/transactions.rb
+++ b/activerecord/lib/active_record/transactions.rb
@@ -126,28 +126,27 @@ module ActiveRecord
     end
 
     def save_with_transactions(perform_validation = true) #:nodoc:
-      rollback_active_record_state { transaction { save_without_transactions(perform_validation) } }
+      rollback_active_record_state! { transaction { save_without_transactions(perform_validation) } }
     end
 
     def save_with_transactions! #:nodoc:
-      rollback_active_record_state { 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
+
+    # 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
-      previous_id = self.id
-      response = yield
-    rescue
-      response = false
-      raise
-    ensure
-      unless response
-        @new_record = previous_new_record
+      yield
+    rescue Exception
+      @new_record = previous_new_record
+      if id_present
         self.id = previous_id
+      else
+        @attributes.delete(self.class.primary_key)
       end
-      response
+      raise
     end
   end
 end
diff --git a/activerecord/test/transactions_test.rb b/activerecord/test/transactions_test.rb
index ecc0da425d..3f3ee1adb7 100644
--- a/activerecord/test/transactions_test.rb
+++ b/activerecord/test/transactions_test.rb
@@ -133,8 +133,9 @@ class TransactionTest < Test::Unit::TestCase
       :content => "Have a nice day",
       :approved => false)
     new_record_snapshot = new_topic.new_record?
+    id_present = new_topic.has_attribute?(Topic.primary_key)
     id_snapshot = new_topic.id
-    
+
     # Make sure the second save gets the after_create callback called.
     2.times do
       begin
@@ -146,6 +147,7 @@ class TransactionTest < Test::Unit::TestCase
         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"
+        assert_equal id_present, new_topic.has_attribute?(Topic.primary_key)
       ensure
         remove_exception_raising_after_create_callback_to_topic
       end