5688: Run callbacks for children within fibers (#5837)

* Run callbacks for children within fibers

* Comments for run child callbacks with around fibers

* Removed the recursive implementation and renamed the fiber implementation to match the original function declaration

* Message, summary, and resolution of invalid around callback implementation error

* Class for invalid around callback error

* try catch block to raise invalid around callbacks error when it is defined without a yield statement (it causes a terminated fiber to be resumed

* Require the invalid around callback error

* Refactored the rescuing of the exception to make it more elegant

* invalid around callback error file is linter-approved

* Fixed indentation for the rescue block

* Spec for the case a user incorrectly defines an around callback without a yield

* raising an invalid around callback error earlier on as soon as a fiber is detected to be dead

* Changed the names of the classes for the invalid around callback spec

* Modified the class names for the around callback without yield to avoid conflicts

* Made the creation fo embedded documents in the around callbacks without yield spec more concise

* Update spec/mongoid/interceptable_spec.rb to use logger

Co-authored-by: Jamis Buck <[email protected]>

* removed the temporary field

---------

Co-authored-by: Jamis Buck <[email protected]>

lib/config/locales/en.yml

@@ -140,6 +140,10 @@ en:
             A collation option is only supported if the query is executed on a MongoDB server
             with version >= 3.4."
           resolution: "Remove the collation option from the query."
+        invalid_around_callback:
+          message: "An around callback must contain a yield in its definition."
+          summary: "The block needs to be yielded to for around callbacks to function as intended."
+          resolution: "Ensure there is a yield statement inside the body of the around callback."
         invalid_async_query_executor:
           message: "Invalid async_query_executor option: %{executor}."
           summary: "A invalid async query executor was specified.

lib/mongoid/errors.rb

@@ -13,6 +13,7 @@
 require "mongoid/errors/immutable_attribute"
 require "mongoid/errors/in_memory_collation_not_supported"
 require "mongoid/errors/invalid_auto_encryption_configuration"
+require "mongoid/errors/invalid_around_callback"
 require "mongoid/errors/invalid_async_query_executor"
 require "mongoid/errors/invalid_collection"
 require "mongoid/errors/invalid_config_file"

lib/mongoid/errors/invalid_around_callback.rb

@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+module Mongoid
+  module Errors
+    # This error is raised when an around callback is
+    # defined by the user without a yield
+    class InvalidAroundCallback < MongoidError
+      # Create the new error.
+      #
+      # @api private
+      def initialize
+        super(compose_message('invalid_around_callback'))
+      end
+    end
+  end
+end

lib/mongoid/interceptable.rb

@@ -161,29 +161,34 @@ def _mongoid_run_child_callbacks(kind, children: nil, &block)
     # Execute the callbacks of given kind for embedded documents including
     # around callbacks.
     #
-    # @note This method is prone to stack overflow errors if the document
-    #   has a large number of embedded documents. It is recommended to avoid
-    #   using around callbacks for embedded documents until a proper solution
-    #   is implemented.
-    #
     # @param [ Symbol ] kind The type of callback to execute.
     # @param [ Array<Document> ] children Children to execute callbacks on. If
     #  nil, callbacks will be executed on all cascadable children of
     #  the document.
     #
     #  @api private
     def _mongoid_run_child_callbacks_with_around(kind, children: nil, &block)
-      child, *tail = (children || cascadable_children(kind))
+      children = (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: with_children, &block)
-      else
-        child.run_callbacks(child_callback_type(kind, child), with_children: with_children) do
-          _mongoid_run_child_callbacks_with_around(kind, children: tail, &block)
+
+      return block&.call if children.empty?
+
+      fibers = children.map do |child|
+        Fiber.new do
+          child.run_callbacks(child_callback_type(kind, child), with_children: with_children) do
+            Fiber.yield
+          end
         end
       end
+
+      fibers.each do |fiber|
+        fiber.resume
+        raise Mongoid::Errors::InvalidAroundCallback unless fiber.alive?
+      end
+
+      block&.call
+
+      fibers.reverse.each(&:resume)
     end
 
     # Execute the callbacks of given kind for embedded documents without

spec/mongoid/interceptable_spec.rb

@@ -2594,4 +2594,35 @@ class TestClass
       end
     end
   end
+
+  context "when around callbacks for embedded children are enabled" do
+    config_override :around_callbacks_for_embeds, true
+
+    context "when around callback is defined without a yield" do
+      class Mother
+        include Mongoid::Document
+        embeds_many :daughters, cascade_callbacks: true
+      end
+
+      class Daughter
+        include Mongoid::Document
+        embedded_in :mother
+        around_save :log_callback
+
+        private
+
+        def log_callback
+          logger.debug('callback invoked')
+        end
+      end
+
+      let(:mom) { Mother.create(daughters: [ Daughter.new, Daughter.new ]) }
+
+      it "raises an InvalidAroundCallback error" do
+        expect do
+          mom.save
+        end.to raise_error(Mongoid::Errors::InvalidAroundCallback)
+      end
+    end
+  end
 end