aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorRyuta Kamizono <kamipo@gmail.com>2018-10-07 15:44:05 +0900
committerRyuta Kamizono <kamipo@gmail.com>2018-10-07 22:42:31 +0900
commit1f8534ca85c32ee26f0e179ac38acf7e61fb0e69 (patch)
tree716f5ace1da34936214961477c9626104e082670 /activerecord
parent651d8743a634837d47070503fde39e97df2e54d0 (diff)
downloadrails-1f8534ca85c32ee26f0e179ac38acf7e61fb0e69.tar.gz
rails-1f8534ca85c32ee26f0e179ac38acf7e61fb0e69.tar.bz2
rails-1f8534ca85c32ee26f0e179ac38acf7e61fb0e69.zip
Fix `AssociationRelation` not to set inverse instance key just like before
Since #31575, `set_inverse_instance` replaces the foreign key by the current owner immediately to make it happen when a record is added to collection association. But `set_inverse_instance` is not only called when a record is added, but also when a record is loaded from queries. And also, that loaded records are not always associated records for some reason (using `or`, `unscope`, `rewhere`, etc). It is hard to distinguish whether or not we should invoke `set_inverse_instance`, but at least we should avoid the undesired side-effect which was brought from #31575. Fixes #34108.
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/lib/active_record/association_relation.rb6
-rw-r--r--activerecord/lib/active_record/associations/association.rb8
-rw-r--r--activerecord/test/cases/associations/has_many_associations_test.rb12
3 files changed, 23 insertions, 3 deletions
diff --git a/activerecord/lib/active_record/association_relation.rb b/activerecord/lib/active_record/association_relation.rb
index 403667fb70..4c538ef2bd 100644
--- a/activerecord/lib/active_record/association_relation.rb
+++ b/activerecord/lib/active_record/association_relation.rb
@@ -31,9 +31,9 @@ module ActiveRecord
private
def exec_queries
- super do |r|
- @association.set_inverse_instance r
- yield r if block_given?
+ super do |record|
+ @association.set_inverse_instance_from_queries(record)
+ yield record if block_given?
end
end
end
diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb
index 44596f4424..e4433356e2 100644
--- a/activerecord/lib/active_record/associations/association.rb
+++ b/activerecord/lib/active_record/associations/association.rb
@@ -103,6 +103,13 @@ module ActiveRecord
record
end
+ def set_inverse_instance_from_queries(record)
+ if inverse = inverse_association_for(record)
+ inverse.inversed_from_queries(owner)
+ end
+ record
+ end
+
# Remove the inverse association, if possible
def remove_inverse_instance(record)
if inverse = inverse_association_for(record)
@@ -114,6 +121,7 @@ module ActiveRecord
self.target = record
@inversed = !!record
end
+ alias :inversed_from_queries :inversed_from
# Returns the class of the target. belongs_to polymorphic overrides this to look at the
# polymorphic_type field on the owner.
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 036df1c31e..45e2aadaf4 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -2356,6 +2356,18 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
assert_equal [accounts(:signals37)], firm.accounts.open
end
+ def test_association_with_or_doesnt_set_inverse_instance_key
+ firm = companies(:first_firm)
+ accounts = firm.accounts.or(Account.where(firm_id: nil)).order(:id)
+ assert_equal [firm.id, nil], accounts.map(&:firm_id)
+ end
+
+ def test_association_with_rewhere_doesnt_inverse_instance_key
+ firm = companies(:first_firm)
+ accounts = firm.accounts.rewhere(firm_id: [firm.id, nil]).order(:id)
+ assert_equal [firm.id, nil], accounts.map(&:firm_id)
+ end
+
test "first_or_initialize adds the record to the association" do
firm = Firm.create! name: "omg"
client = firm.clients_of_firm.first_or_initialize