From cc405496ce85ee0073268baefdb2be5d4b062f91 Mon Sep 17 00:00:00 2001
From: Ben Woosley <ben.woosley@gmail.com>
Date: Tue, 18 Jun 2013 15:10:48 -0700
Subject: [PATCH] =?UTF-8?q?Fix=20that=20a=20collection=20proxy=20could=20b?=
 =?UTF-8?q?e=20cached=20before=20the=20save=20of=20the=20owner,=20resultin?=
 =?UTF-8?q?g=20in=20an=20invalid=20proxy=20lacking=20the=20owner=E2=80=99s?=
 =?UTF-8?q?=20id.?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Absent this fix calls like: owner.association.update_all to behave unexpectedly because they try to act on association objects where
owner_id is null.

more evidence here: https://gist.github.com/Empact/5865555
```
Active Record 3.2.13
-- create_table(:firms, {:force=>true})
   -> 0.1371s
-- create_table(:clients, {:force=>true})
   -> 0.0005s
1 clients. 1 expected.
1 clients updated. 1 expected.
```

```
Active Record 4.0.0
-- create_table(:firms, {:force=>true})
   -> 0.1606s
-- create_table(:clients, {:force=>true})
   -> 0.0004s
1 clients. 1 expected.
0 clients updated. 1 expected.
```
---
 activerecord/CHANGELOG.md                                 | 6 ++++++
 .../active_record/associations/collection_association.rb  | 8 +++++++-
 .../test/cases/associations/has_many_associations_test.rb | 7 +++++++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 210151a2ad..92e1575484 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   Cache CollectionAssociation#reader proxies separately before and after
+    the owner has been saved so that the proxy is not cached without the
+    owner's id.
+
+    *Ben Woosley*
+
 *   Honor overridden `rack.test` in Rack environment for the connection
     management middleware.
 
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 1836ff0910..c5dff7ee1a 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -33,7 +33,13 @@ module ActiveRecord
           reload
         end
 
-        @proxy ||= CollectionProxy.create(klass, self)
+        if owner.new_record?
+          # 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)
+        else
+          @proxy ||= CollectionProxy.create(klass, self)
+        end
       end
 
       # Implements the writer method, e.g. foo.items= for Foo.has_many :items
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index e34b993029..c72d9342a9 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -387,6 +387,13 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal "Summit", Firm.all.merge!(:order => "id").first.clients_using_primary_key.first.name
   end
 
+  def test_update_all_on_association_accessed_before_save
+    firm = Firm.new(name: 'Firm')
+    firm.clients << Client.first
+    firm.save!
+    assert_equal firm.clients.count, firm.clients.update_all(description: 'Great!')
+  end
+
   def test_belongs_to_sanity
     c = Client.new
     assert_nil c.firm, "belongs_to failed sanity check on new object"