MONGOID-5542 Prevent multiple calls of embedded docs callbacks (#5621) (

#5626)

* Add test

* Fix double callback call

* Change test that expects the old behavior

* Add config option

* Flip the flag

lib/mongoid/config.rb

@@ -142,6 +142,16 @@ module Config
       end
     end
 
+    # When this flag is true, callbacks for every embedded document will be
+    # called only once, even if the embedded document is embedded in multiple
+    # documents in the root document's dependencies graph.
+    # This will be the default in 9.0. Setting this flag to false restores the
+    # pre-9.0 behavior, where callbacks are called for every occurrence of an
+    # embedded document. The pre-9.0 behavior leads to a problem that for multi
+    # level nested documents callbacks are called multiple times.
+    # See https://jira.mongodb.org/browse/MONGOID-5542
+    option :prevent_multiple_calls_of_embedded_callbacks, default: false
+
     # When this flag is true, callbacks for embedded documents will not be
     # called. This is the default in 8.x, but will be changed to false in 9.0.
     #

lib/mongoid/interceptable.rb

@@ -167,13 +167,14 @@ def _mongoid_run_child_callbacks(kind, children: nil, &block)
     #  @api private
     def _mongoid_run_child_callbacks_with_around(kind, children: nil, &block)
       child, *tail = (children || cascadable_children(kind))
+      with_children = !Mongoid::Config.prevent_multiple_calls_of_embedded_callbacks
       if child.nil?
         block&.call
       elsif tail.empty?
-        child.run_callbacks(child_callback_type(kind, child), with_children: false, &block)
+        child.run_callbacks(child_callback_type(kind, child), with_children: with_children, &block)
       else
-        child.run_callbacks(child_callback_type(kind, child), with_children: false) do
-          _mongoid_run_child_callbacks_with_around(kind, children: tail, &block)
+        child.run_callbacks(child_callback_type(kind, child), with_children: with_children) do
+          _mongoid_run_child_callbacks(kind, children: tail, &block)
         end
       end
     end

spec/integration/callbacks_models.rb

@@ -153,3 +153,40 @@ class Building
 
   has_and_belongs_to_many :architects, dependent: :nullify
 end
+
+class Root
+  include Mongoid::Document
+  embeds_many :embedded_once, cascade_callbacks: true
+  after_save :trace
+
+  attr_accessor :logger
+
+  def trace
+    logger << :root
+  end
+end
+
+class EmbeddedOnce
+  include Mongoid::Document
+  embeds_many :embedded_twice, cascade_callbacks: true
+  embedded_in :root
+  after_save :trace
+
+  attr_accessor :logger
+
+  def trace
+    logger << :embedded_once
+  end
+end
+
+class EmbeddedTwice
+  include Mongoid::Document
+  embedded_in :embedded_once
+  after_save :trace
+
+  attr_accessor :logger
+
+  def trace
+    logger << :embedded_twice
+  end
+end

spec/integration/callbacks_spec.rb

@@ -467,4 +467,31 @@ class PreviouslyPersistedPerson
       expect { book.save! }.not_to raise_error(SystemStackError)
     end
   end
+
+  context 'nested embedded documents' do
+    config_override :prevent_multiple_calls_of_embedded_callbacks, true
+
+    let(:logger) { Array.new }
+
+    let(:root) do
+      Root.new(
+        embedded_once: [
+          EmbeddedOnce.new(
+            embedded_twice: [EmbeddedTwice.new]
+          )
+        ]
+      )
+    end
+
+    before(:each) do
+      root.logger = logger
+      root.embedded_once.first.logger = logger
+      root.embedded_once.first.embedded_twice.first.logger = logger
+    end
+
+    it 'runs callbacks in the correct order' do
+      root.save!
+      expect(logger).to eq(%i[embedded_twice embedded_once root])
+    end
+  end
 end

spec/mongoid/interceptable_spec.rb

@@ -392,6 +392,8 @@ class TestClass
     context 'with embedded grandchildren' do
       IS = InterceptableSpec
 
+      config_override :prevent_multiple_calls_of_embedded_callbacks, true
+
       context 'when creating' do
         let(:registry) { IS::CallbackRegistry.new(only: %i[ before_save ]) }
 
@@ -656,6 +658,26 @@ class TestClass
           band.notes.push(note)
           record.notes.push(note)
         end
+
+        context "when saving the root" do
+          context 'with prevent_multiple_calls_of_embedded_callbacks enabled' do
+            config_override :prevent_multiple_calls_of_embedded_callbacks, true
+
+            it "executes the callbacks only once for each document" do
+              expect(note).to receive(:update_saved).once
+              band.save!
+            end
+          end
+
+          context 'with prevent_multiple_calls_of_embedded_callbacks disabled' do
+            config_override :prevent_multiple_calls_of_embedded_callbacks, false
+
+            it "executes the callbacks once for each ember" do
+              expect(note).to receive(:update_saved).twice
+              band.save!
+            end
+          end
+        end
       end
     end