aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorBen Woosley <ben.woosley@gmail.com>2013-06-18 15:10:48 -0700
committerBen Woosley <pair+ben@pacerpro.com>2014-10-03 03:04:16 -0700
commitcc405496ce85ee0073268baefdb2be5d4b062f91 (patch)
tree9a7721c2b03e95b01faacafe89782c1fea5c9d45 /activerecord
parentde50a91daad90bf2a0e6bece60a1c2211ade4682 (diff)
downloadrails-cc405496ce85ee0073268baefdb2be5d4b062f91.tar.gz
rails-cc405496ce85ee0073268baefdb2be5d4b062f91.tar.bz2
rails-cc405496ce85ee0073268baefdb2be5d4b062f91.zip
Fix that a collection proxy could be cached before the save of the owner, resulting in an invalid proxy lacking the owner’s id.
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. ```
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md6
-rw-r--r--activerecord/lib/active_record/associations/collection_association.rb8
-rw-r--r--activerecord/test/cases/associations/has_many_associations_test.rb7
3 files changed, 20 insertions, 1 deletions
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"