1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Ensure has_one autosave association callbacks get called once

When saving a record, autosave adds callbacks to save its' associations.
Since the associations can have similar callbacks for the inverse,
endless loops could occur.

To prevent these endless loops, the callbacks for `has_many` and
`belongs_to` are defined as methods that only execute once.
This is implemented in the `define_non_cyclic_method` method.
However, this wasn't used for the `has_one` callbacks.

While `has_one` association callbacks didn't result in endless loops,
they could execute multiple times.
For example for a bidirectional `has_one` with autosave enabled,
the `save_has_one_association` gets called twice:

    class Pirate < ActiveRecord::Base
      has_one :ship, autosave: true

      def save_has_one_association(reflection)
        @count ||= 0
        @count += 1 if reflection.name == :ship
        super
      end
    end

    class Ship < ActiveRecord::Base
      belongs_to :pirate, autosave: true
    end

    pirate = Pirate.new(catchphrase: "Aye")
    pirate.build_ship(name: "Nights Dirty Lightning")
    pirate.save!
    # this returns 2 instead of 1.
    assert_equal 1, pirate.instance_variable_get(:@count)

This commit changes `has_one` autosave callbacks to be non-cyclic as
well. By doing this the autosave callback are made more consistent for
all 3 cases: `has_many`, `has_one` and `belongs_to`.
This commit is contained in:
Petrik 2021-04-14 21:16:35 +02:00
parent 80a23227ea
commit 2a786431e2
3 changed files with 59 additions and 1 deletions

View file

@ -1,3 +1,11 @@
* Ensure has_one autosave association callbacks get called once.
Change the `has_one` autosave callback to be non cyclic as well.
By doing this the autosave callback are made more consistent for
all 3 cases: `has_many`, `has_one` and `belongs_to`.
*Petrik de Heus*
* Only update dirty attributes once for cyclic autosave callbacks.
Instead of calling `changes_applied` everytime `save` is called,

View file

@ -196,7 +196,7 @@ module ActiveRecord
after_create save_method
after_update save_method
elsif reflection.has_one?
define_method(save_method) { save_has_one_association(reflection) } unless method_defined?(save_method)
define_non_cyclic_method(save_method) { save_has_one_association(reflection) }
# Configures two callbacks instead of a single after_save so that
# the model may rely on their execution order relative to its
# own callbacks.

View file

@ -87,6 +87,56 @@ class TestAutosaveAssociationsInGeneral < ActiveRecord::TestCase
assert_predicate r, :valid?
end
def test_autosave_collection_association_callbacks_get_called_once
ship_with_saving_stack = Class.new(Ship) do
def save_collection_association(reflection)
@count ||= 0
@count += 1 if reflection.name == :parts
super
end
end
ship = ship_with_saving_stack.new(name: "Nights Dirty Lightning")
ship.parts.build(name: "part")
ship.save!
assert_equal 1, ship.instance_variable_get(:@count)
end
def test_autosave_has_one_association_callbacks_get_called_once
# a bidirectional autosave is required to trigger multiple calls to
# save_has_one_association
assert Ship.reflect_on_association(:pirate).options[:autosave]
assert Pirate.reflect_on_association(:ship).options[:autosave]
pirate_with_saving_stack = Class.new(Pirate) do
def save_has_one_association(reflection)
@count ||= 0
@count += 1 if reflection.name == :ship
super
end
end
pirate = pirate_with_saving_stack.new(catchphrase: "Aye")
pirate.build_ship(name: "Nights Dirty Lightning")
pirate.save!
assert_equal 1, pirate.instance_variable_get(:@count)
end
def test_autosave_belongs_to_association_callbacks_get_called_once
ship_with_saving_stack = Class.new(Ship) do
def save_belongs_to_association(reflection)
@count ||= 0
@count += 1 if reflection.name == :pirate
super
end
end
ship = ship_with_saving_stack.new(name: "Nights Dirty Lightning")
ship.build_pirate(catchphrase: "Aye")
ship.save!
assert_equal 1, ship.instance_variable_get(:@count)
end
def test_should_not_add_the_same_callbacks_multiple_times_for_has_one
assert_no_difference_when_adding_callbacks_twice_for Pirate, :ship
end