From 268a5bb010ef91880d9b03b60e5e86ae5521e73c Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Thu, 29 Sep 2016 14:57:31 -0400 Subject: Don't skip in-memory insertion of associations when loaded in validate This was caused by 6d0d83a33f59d9415685852cf77818c41e2e2700. While the bug it's trying to fix is handled if the association is loaded in an after_(create|save) callback, it doesn't handle any cases that load the association before the persistence takes place (validation, or before_* filters). Instead of caring about the timing of persistence, we can just ensure that we're not double adding the record instead. The test from that commit actually broke, but it was not because the bug has been re-introduced. It was because `Bulb` in our test suite is doing funky things that look like STI but isn't STI, so equality comparison didn't happen as the loaded model was of a different class. Fixes #26661. --- .../associations/has_many_associations_test.rb | 34 ++++++++++++++++++++-- 1 file changed, 31 insertions(+), 3 deletions(-) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index fed59c2ab3..0ce67f971b 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -2461,9 +2461,12 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end test "double insertion of new object to association when same association used in the after create callback of a new object" do - car = Car.create! - car.bulbs << TrickyBulb.new - assert_equal 1, car.bulbs.size + reset_callbacks(:save, Bulb) do + Bulb.after_save { |record| record.car.bulbs.to_a } + car = Car.create! + car.bulbs << Bulb.new + assert_equal 1, car.bulbs.size + end end def test_association_force_reload_with_only_true_is_deprecated @@ -2510,9 +2513,34 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_no_queries { car.bulb_ids } end + def test_loading_association_in_validate_callback_doesnt_affect_persistence + reset_callbacks(:validation, Bulb) do + Bulb.after_validation { |m| m.car.bulbs.load } + + car = Car.create!(name: "Car") + bulb = car.bulbs.create! + + assert_equal [bulb], car.bulbs + end + end + private def force_signal37_to_load_all_clients_of_firm companies(:first_firm).clients_of_firm.load_target end + + def reset_callbacks(kind, klass) + old_callbacks = {} + old_callbacks[klass] = klass.send("_#{kind}_callbacks").dup + klass.subclasses.each do |subclass| + old_callbacks[subclass] = subclass.send("_#{kind}_callbacks").dup + end + yield + ensure + klass.send("_#{kind}_callbacks=", old_callbacks[klass]) + klass.subclasses.each do |subclass| + subclass.send("_#{kind}_callbacks=", old_callbacks[subclass]) + end + end end -- cgit v1.2.3