1 files changed, 47 insertions, 42 deletions

diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 77282e6463..a49fb155ee 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -30,7 +30,8 @@ module ActiveRecord
           reload
         end
 
-        CollectionProxy.create(klass, self)
+        @proxy ||= CollectionProxy.create(klass, self)
+        @proxy.reset_scope
       end
 
       # Implements the writer method, e.g. foo.items= for Foo.has_many :items
@@ -43,10 +44,7 @@ module ActiveRecord
         if loaded?
           target.pluck(reflection.association_primary_key)
         else
-          @association_ids ||= (
-            column = "#{reflection.quoted_table_name}.#{reflection.association_primary_key}"
-            scope.pluck(column)
-          )
+          @association_ids ||= scope.pluck(reflection.association_primary_key)
         end
       end
 
@@ -68,6 +66,7 @@ module ActiveRecord
       def reset
         super
         @target = []
+        @association_ids = nil
       end
 
       def find(*args)
@@ -280,35 +279,6 @@ module ActiveRecord
         replace_on_target(record, index, skip_callbacks, &block)
       end
 
-      def replace_on_target(record, index, skip_callbacks)
-        callback(:before_add, record) unless skip_callbacks
-
-        begin
-          if index
-            record_was = target[index]
-            target[index] = record
-          else
-            target << record
-          end
-
-          set_inverse_instance(record)
-
-          yield(record) if block_given?
-        rescue
-          if index
-            target[index] = record_was
-          else
-            target.delete(record)
-          end
-
-          raise
-        end
-
-        callback(:after_add, record) unless skip_callbacks
-
-        record
-      end
-
       def scope
         scope = super
         scope.none! if null_scope?
@@ -328,13 +298,14 @@ module ActiveRecord
       private
 
         def find_target
-          return scope.to_a if skip_statement_cache?
+          scope = self.scope
+          return scope.to_a if skip_statement_cache?(scope)
 
           conn = klass.connection
           sc = reflection.association_scope_cache(conn, owner) do
             StatementCache.create(conn) { |params|
               as = AssociationScope.create { params.bind }
-              target_scope.merge as.scope(self, conn)
+              target_scope.merge!(as.scope(self))
             }
           end
 
@@ -385,15 +356,22 @@ module ActiveRecord
             transaction do
               add_to_target(build_record(attributes)) do |record|
                 yield(record) if block_given?
-                insert_record(record, true, raise)
+                insert_record(record, true, raise) {
+                  @_was_loaded = loaded?
+                  @association_ids = nil
+                }
               end
             end
           end
         end
 
         # Do the relevant stuff to insert the given record into the association collection.
-        def insert_record(record, validate = true, raise = false)
-          raise NotImplementedError
+        def insert_record(record, validate = true, raise = false, &block)
+          if raise
+            record.save!(validate: validate, &block)
+          else
+            record.save(validate: validate, &block)
+          end
         end
 
         def create_scope
@@ -448,19 +426,46 @@ module ActiveRecord
           end
         end
 
-        def concat_records(records, should_raise = false)
+        def concat_records(records, raise = false)
           result = true
 
           records.each do |record|
             raise_on_type_mismatch!(record)
-            add_to_target(record) do |rec|
-              result &&= insert_record(rec, true, should_raise) unless owner.new_record?
+            add_to_target(record) do
+              unless owner.new_record?
+                result &&= insert_record(record, true, raise) {
+                  @_was_loaded = loaded?
+                  @association_ids = nil
+                }
+              end
             end
           end
 
           result && records
         end
 
+        def replace_on_target(record, index, skip_callbacks)
+          callback(:before_add, record) unless skip_callbacks
+
+          set_inverse_instance(record)
+
+          @_was_loaded = true
+
+          yield(record) if block_given?
+
+          if index
+            target[index] = record
+          elsif @_was_loaded || !loaded?
+            target << record
+          end
+
+          callback(:after_add, record) unless skip_callbacks
+
+          record
+        ensure
+          @_was_loaded = nil
+        end
+
         def callback(method, record)
           callbacks_for(method).each do |callback|
             callback.call(method, owner, record)