Ensure not to load the entire association when bulk updating existing records using nested attributes

2 files changed, 22 insertions, 12 deletions

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 6a4cef0d50..0dfd966466 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -451,6 +451,16 @@ module ActiveRecord
           records
         end
 
+        def add_record_to_target_with_callbacks(record)
+          callback(:before_add, record)
+          yield(record) if block_given?
+          @target ||= [] unless loaded?
+          @target << record unless @reflection.options[:uniq] && @target.include?(record)
+          callback(:after_add, record)
+          set_inverse_instance(record, @owner)
+          record
+        end
+
       private
         def create_record(attrs)
           attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash)
@@ -475,16 +485,6 @@ module ActiveRecord
           end
         end
 
-        def add_record_to_target_with_callbacks(record)
-          callback(:before_add, record)
-          yield(record) if block_given?
-          @target ||= [] unless loaded?
-          @target << record unless @reflection.options[:uniq] && @target.include?(record)
-          callback(:after_add, record)
-          set_inverse_instance(record, @owner)
-          record
-        end
-
         def remove_records(*records)
           records = flatten_deeper(records)
           records.each { |record| raise_on_type_mismatch(record) }
diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb
index ee2d420194..70a460d41d 100644
--- a/activerecord/lib/active_record/nested_attributes.rb
+++ b/activerecord/lib/active_record/nested_attributes.rb
@@ -348,14 +348,24 @@ module ActiveRecord
         attributes_collection = attributes_collection.sort_by { |index, _| index.to_i }.map { |_, attributes| attributes }
       end
 
+      association = send(association_name)
+
+      existing_records = if association.loaded?
+        association.to_a
+      else
+        attribute_ids = attributes_collection.map {|a| a['id'] || a[:id] }.compact
+        attribute_ids.present? ? association.all(:conditions => {:id => attribute_ids}) : []
+      end
+
       attributes_collection.each do |attributes|
         attributes = attributes.with_indifferent_access
 
         if attributes['id'].blank?
           unless reject_new_record?(association_name, attributes)
-            send(association_name).build(attributes.except(*UNASSIGNABLE_KEYS))
+            association.build(attributes.except(*UNASSIGNABLE_KEYS))
           end
-        elsif existing_record = send(association_name).detect { |record| record.id.to_s == attributes['id'].to_s }
+        elsif existing_record = existing_records.detect { |record| record.id.to_s == attributes['id'].to_s }
+          association.send(:add_record_to_target_with_callbacks, existing_record) unless association.loaded?
           assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy])
         else
           raise_nested_attributes_record_not_found(association_name, attributes['id'])