Merge pull request #19348 from Empact/null-scope

Reuse the CollectionAssociation#reader proxy cache if the foreign key is present from the start.

Conflicts:
	activerecord/CHANGELOG.md

3 files changed, 19 insertions, 2 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 07714538fb..89cea5a7cb 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   Reuse the `CollectionAssociation#reader` cache when the foreign key is
+    available prior to save.
+
+    *Ben Woosley*
+
 *   Add `config.active_record.dump_schemas` to fix `db:structure:dump`
     when using schema_search_path and PostgreSQL extensions.
 
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 1e245926e0..88531205a1 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -33,10 +33,10 @@ module ActiveRecord
           reload
         end
 
-        if owner.new_record?
+        if null_scope?
           # Cache the proxy separately before the owner has an id
           # or else a post-save proxy will still lack the id
-          @new_record_proxy ||= CollectionProxy.create(klass, self)
+          @null_proxy ||= CollectionProxy.create(klass, self)
         else
           @proxy ||= CollectionProxy.create(klass, self)
         end
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 675bed9bfa..290b2a0d6b 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -532,9 +532,21 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_update_all_on_association_accessed_before_save
     firm = Firm.new(name: 'Firm')
+    clients_proxy_id = firm.clients.object_id
     firm.clients << Client.first
     firm.save!
     assert_equal firm.clients.count, firm.clients.update_all(description: 'Great!')
+    assert_not_equal clients_proxy_id, firm.clients.object_id
+  end
+
+  def test_update_all_on_association_accessed_before_save_with_explicit_foreign_key
+    # We can use the same cached proxy object because the id is available for the scope
+    firm = Firm.new(name: 'Firm', id: 100)
+    clients_proxy_id = firm.clients.object_id
+    firm.clients << Client.first
+    firm.save!
+    assert_equal firm.clients.count, firm.clients.update_all(description: 'Great!')
+    assert_equal clients_proxy_id, firm.clients.object_id
   end
 
   def test_belongs_to_sanity