From dfaad4f5a78b2e21a3b86e0daea3afb3b483dc5e Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Tue, 31 May 2011 20:32:09 +0100
Subject: Only save the record once when calling create! on a collection
 association. Fixes #1360.

---
 .../associations/collection_association.rb         | 40 ++++++++++++----------
 .../has_and_belongs_to_many_association.rb         | 10 ++++--
 .../associations/has_many_association.rb           |  9 +++--
 .../associations/has_many_through_association.rb   | 11 ++++--
 4 files changed, 45 insertions(+), 25 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index d0ab6b4b86..02cc455a4e 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -104,26 +104,11 @@ module ActiveRecord
       end
 
       def create(attributes = {}, options = {}, &block)
-        unless owner.persisted?
-          raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved"
-        end
-
-        if attributes.is_a?(Array)
-          attributes.collect { |attr| create(attr, options, &block) }
-        else
-          transaction do
-            add_to_target(build_record(attributes, options)) do |record|
-              yield(record) if block_given?
-              insert_record(record)
-            end
-          end
-        end
+        create_record(attributes, options, &block)
       end
 
-      def create!(attrs = {}, options = {}, &block)
-        record = create(attrs, options, &block)
-        Array.wrap(record).each(&:save!)
-        record
+      def create!(attributes = {}, options = {}, &block)
+        create_record(attributes, options, true, &block)
       end
 
       # Add +records+ to this association. Returns +self+ so method calls may be chained.
@@ -422,8 +407,25 @@ module ActiveRecord
           persisted + memory
         end
 
+        def create_record(attributes, options, raise = false, &block)
+          unless owner.persisted?
+            raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved"
+          end
+
+          if attributes.is_a?(Array)
+            attributes.collect { |attr| create_record(attr, options, raise, &block) }
+          else
+            transaction do
+              add_to_target(build_record(attributes, options)) do |record|
+                yield(record) if block_given?
+                insert_record(record, true, raise)
+              end
+            end
+          end
+        end
+
         # Do the relevant stuff to insert the given record into the association collection.
-        def insert_record(record, validate = true)
+        def insert_record(record, validate = true, raise = false)
           raise NotImplementedError
         end
 
diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
index 217213808b..f7ce70db1a 100644
--- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -9,8 +9,14 @@ module ActiveRecord
         super
       end
 
-      def insert_record(record, validate = true)
-        return if record.new_record? && !record.save(:validate => validate)
+      def insert_record(record, validate = true, raise = false)
+        if record.new_record?
+          if raise
+            record.save!(:validate => validate)
+          else
+            return unless record.save(:validate => validate)
+          end
+        end
 
         if options[:insert_sql]
           owner.connection.insert(interpolate(options[:insert_sql], record))
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index 7172e89a05..50ee60284c 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -7,9 +7,14 @@ module ActiveRecord
     # is provided by its child HasManyThroughAssociation.
     class HasManyAssociation < CollectionAssociation #:nodoc:
 
-      def insert_record(record, validate = true)
+      def insert_record(record, validate = true, raise = false)
         set_owner_attributes(record)
-        record.save(:validate => validate)
+
+        if raise
+          record.save!(:validate => validate)
+        else
+          record.save(:validate => validate)
+        end
       end
 
       private
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index 7708228d23..280688be52 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -33,9 +33,16 @@ module ActiveRecord
         super
       end
 
-      def insert_record(record, validate = true)
+      def insert_record(record, validate = true, raise = false)
         ensure_not_nested
-        return if record.new_record? && !record.save(:validate => validate)
+
+        if record.new_record?
+          if raise
+            record.save!(:validate => validate)
+          else
+            return unless record.save(:validate => validate)
+          end
+        end
 
         through_record(record).save!
         update_counter(1)
-- 
cgit v1.2.3